Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP Server #726

Merged
merged 13 commits into from
Dec 29, 2020
Merged

HTTP Server #726

merged 13 commits into from
Dec 29, 2020

Conversation

aarroyoc
Copy link
Contributor

@aarroyoc aarroyoc commented Dec 9, 2020

First version of the pure Prolog HTTP server

Sample app:

:- use_module(library(http/http_server)).
:- use_module(library(lists)).
:- use_module(library(dcgs)).
:- use_module(library(format)).

text_handler(Request, Response) :-
    http_status_code(Response, 200),
    http_body(Response, text("Welcome to Scryer Prolog!")).

sample_handler(Request, Response) :-
    http_headers(Request, Headers),
    member("user-agent"-UserAgent, Headers),
    http_body(Response, text(UserAgent)).

sample_body_handler(Request, Response) :-
    http_body(Request, binary(Body)),
    http_body(Response, binary(Body)),
    http_headers(Response, ["content-type"-"application/json"]).

text_echo_handler(Request, Response) :-
    http_body(Request, text(TextBody)),
    http_body(Response, text(TextBody)).

parameter_handler(User, Request, Response) :-
    http_body(Response, text(User)).

redirect(Request, Response) :-
    http_redirect(Response, "/").

search(Request, Response) :-
    http_query(Request, "q", SearchTerm),
    phrase(format_("Search term: ~s", [SearchTerm]), ResponseText),
    http_body(Response, text(ResponseText)).

file(Request, Response) :-
    http_body(Response, file('/home/aarroyoc/dev/scryer-http-test/comuneros.jpg')). % Put your own file

run :-
    http_listen(7890, [
        get('', text_handler),
        get('user-agent', sample_handler),
        post(echo, sample_body_handler),
        post('echo-text', text_echo_handler),
        get(user/User, parameter_handler(User)),
        get(redirectme, redirect),
        get(search, search),
        get(file, file)
    ]).

Run:

scryer-prolog -g 'run.' sampleapp.pl

@triska
Copy link
Contributor

triska commented Dec 9, 2020

Very nice, thank you a lot for working on this!

You can use setup_call_cleanup/3 from library(iso_ext) to ensure that a stream is reliably closed, also if an error or failure occurs. The pattern is:

setup_call_cleanup(<open stream>, Goal, <close stream>)

@aarroyoc
Copy link
Contributor Author

Thanks for your help, I'll look into it ASAP. However, I've hit two strange things (I don't know if they're bugs or it's just my ignorance) that I want to ask:

  • It seems that get_bytes (I've taken the code from an example) doesn't work in Socket Streams because they don't end, they just block until forever. I don't know what is the solution here.
  • The server doesn't loop and I think, I can't make it loop even by asking for more answers in the CLI. I think it is because server_socket_accept doesn't allow multiple solutions (to give different streams)

Any help with these two points will be very useful!

@triska
Copy link
Contributor

triska commented Dec 12, 2020

Regarding get_bytes/2: Yes, it will simply wait for more input, because conceivably more input can still arrive from the client since the connection is not closed. However, we know from the way the HTTP protocol works that only as many bytes need to be waited for as the Content-Length header field says (I hope that's correct, and there may be exceptions to this rule, please see the HTTP specification RFC 2616 for definite answers!). So, I think one solution for this case is to take that header field into account. Note also that depending on the content-type and encoding, it could be useful to switch the type and encoding of the stream, a good way to solve this is outlined in #614.

Regarding server_socket_accept/4, this is how it is defined: https://sicstus.sics.se/sicstus/docs/4.3.2/html/sicstus/lib_002dsockets.html

You can simply call it again to accept the next incoming connection. A general accept loop may look roughly like this:

server(IP, Port) :-
        socket_server_open(IP:Port, Socket),
        accept_loop(Socket).

accept_loop(Socket) :-
        format("waiting for connections...~n", []),
        setup_call_cleanup(socket_server_accept(Socket, Client, Stream, [type(binary)]),
                           (   format("handling client ~q...~n", [Client]),
                               request_response(Stream)
                           ),
                           close(Stream)),
        accept_loop(Socket).

Source: https://github.com/triska/the-power-of-prolog/blob/master/server.pl

In a more general setting of a configurable HTTP server with dynamic dispatch rules, I think it would be useful if each of the configured HTTP handlers also gets passed the parsed header contents of the request, so that the header fields can be taken into account when formulating the response.

I hope this helps! Keep up the great work!

@aarroyoc
Copy link
Contributor Author

According to the HTTP/1.0 standard, Content-Length is always mandatory when there's a body (in HTTP/1.1, there's an alternative for chunks, but let's start with something easier). Lots of thanks!

With the setup_call_cleanup I think I found a bug because when it succeeds, it doesn't call the cleanup clause. Tomorrow I'll try a minimal-example-not-so-minimal (because minimal examples work). For now, I had to put close(Stream) in both the goal and the cleanup clause, and works without trouble (which I think is wrong but it works because it doesn't close the Stream twice strangely).

Anyway, thanks for your help! I think we will have an HTTP/1.0 server very soon.

@triska
Copy link
Contributor

triska commented Dec 14, 2020

The cleanup is performed at the earliest opportunity: That is, after the goal fails, raises an exception, or succeeds deterministically.

As long as choice-points remain, the cleanup is not performed, because the information from the setup part (such as a stream handle) may still be needed then.

Currently, Scryer Prolog leaves many redundant choice-points (see also #192), and these prevent the cleanup to be performed. You can work around this, for example using once/1, but I would not recommend it: Rather write the code as elegantly as possible, and hope for #192 or at least a somewhat better indexing scheme to become available in the future.

@aarroyoc
Copy link
Contributor Author

I'm doing a test suite to make sure the code is working and no regressions are found, using a non-Prolog HTTP client implementation: https://github.com/aarroyoc/scryer-http-test

Maybe in a future this test suite can be integrated into Scryer tests

@triska
Copy link
Contributor

triska commented Dec 20, 2020

Thank you a lot! Meanwhile, @notoria has implemented #732 to obtain better indexing, and I hope this solves the issue with cleanup you mentioned, please have a look.

put_bytes(Stream, BinaryResponse).

write_headers(Stream, Headers) :-
forall(member(Key-Value, Headers), format(Stream, "~s: ~s\r\n", [Key, Value])).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using forall/2 is not good style, because the side-effects this produces cannot be tested easily (i.e., via Prolog queries).

A better approach in this example would be to write a DCG that declaratively describes the header, as a list of characters. You can use the non-terminal format_//2 in the description .

When the header is completely described (phrase(header(...), Cs)), use a single format/3 call to write the entire list of characters to the stream: format(Stream, "~s", Cs).

overwrite_header("Connection"-"Close", Headers0, Headers1),
write_headers(Stream, Headers1),
format(Stream, "\r\n", []),
open(Filename, read, FileStream, [type(binary)]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open/[3,4] is best used in connection with setup_call_cleanup/3 to reliably close the stream.

For instance, in this case:

setup_call_cleanup(open(FileName, read, FileStream, [type(binary)]), 
                   pipe_bytes(FileStream,Stream),
                   close(FileStream))

Comment on lines +140 to +141
phrase(header(Headers), Cs),
format(Stream, "~s", [Cs]).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good improvement, thank you a lot!

Once phrase_to_stream/2 (see #691) becomes available, it can be used to directly write the list of characters to the stream as soon as they become known, without manifesting the list in memory.

send_response(Stream, http_response(StatusCode0, file(Filename), Headers)) :-
default(StatusCode0, 200, StatusCode),
format(Stream, "HTTP/1.0 ~d\r\n", [StatusCode]),
overwrite_header("Connection"-"Close", Headers0, Headers1),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm still learning how to read Prolog, but is it a mistake that this is the only use of Headers0 in this rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a mistake 🤣 Nice catch!

@aarroyoc
Copy link
Contributor Author

I think that the only thing missing for this first version is a correct URL Encode parser, which I'll try to do tomorrow. With that, we will be able to read some forms also, but I think that should be in another PR.

asserta(http_handler(options, Path, Handler)),
register_handlers([Handler|Handlers]) :-
Handler =.. [Method, Path, Closure],
asserta(http_handler(Method, Path, Closure)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider passing around the handlers as arguments instead of asserting clauses dynamically: Asserting clauses can have unintended effects for example when spawning multiple servers in parallel or consecutively, and complicates testing and debugging the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know but then I got into a problem I don't know how to solve. In the route matching I'm using variables, but if I use them as they're on the list, it only works for the first time, because later the value keeps unified in the list with the concrete request data.

Let me explain with a sample. If I have:

get(user/User, parameter(User))

as a handler and I store it in a list, the first time (GET /usr/mthom) User will unify with "mthom" for example, and we will call parameter("mthom", Request, Response). The second time, User will still be "mthom" because they're in the same list, so doing a GET /user/triska will fail because "triska" = "mthom". I want to keep the variable for every request, no matter how many times has been called before and by passing a list I don't know how to do it.

Maybe it's impossible and I need to think in another syntax for this type or parameters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use copy_term/2 to copy the entire list of handlers, thus using fresh variables each time you are using the rules. For example:

?- copy_term(X/Y-Y, Copy).
   Copy = _A/_B-_B.

% languages. Since HTTP internals are ASCII, this is fine for this usecase.
chars_lower(Chars, Lower) :-
maplist(char_lower, Chars, Lower).
char_lower(Char, Lower) :-
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For later, this functionality would be a useful and very welcome extension to char_type/2 from library(charsio), for example as:

?- char_type(X, to_lower('A')).
X = a.

Using appropriate Rust features for reasoning about Unicode, this could likely be made efficient and also general.

@aarroyoc aarroyoc changed the title [WIP] HTTP Server HTTP Server Dec 28, 2020
@aarroyoc aarroyoc marked this pull request as ready for review December 28, 2020 13:47
@ghost
Copy link

ghost commented Dec 28, 2020

The predicate urldecode//1 could be url_decode//1. There are some singleton variables:

$ scryer-prolog http_server.pl 
Warning: http_server.pl:86: Singleton variables: [Client, Version]
?-

You can remove the warning by adding an underscore like _Client.

@mthom mthom merged commit b8020db into mthom:master Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants