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

evhttp_request->uri isn't a uri... #42

Closed
TheRook opened this issue Feb 26, 2013 · 6 comments
Closed

evhttp_request->uri isn't a uri... #42

TheRook opened this issue Feb 26, 2013 · 6 comments

Comments

@TheRook
Copy link

TheRook commented Feb 26, 2013

A Uniform Resource Identifier consists of the following components:
[scheme:][//authority][path][?query][#fragment]

evhttp_request->uri returns the [path] and [?query] components concatenated together... This is annoying because If you try to access an evhttp by typing the following URI in the address bar of a browser:
http://localhost:1234/?key=var

Then evhttp_request->uri returns "/?key=var" which causes the following code to return a dictionary with the key element "/?key" instead of "key":
evhttp_parse_query_str(req->uri, &GET);

The solution is probably to remove evhttp_request->uri entirely and replace it with evhttp_request->path and evhttp_request->query.

For example to obtain the GET prams you could use this:
evhttp_parse_query_str(req->query, &GET);

And if you wanted to work with the path:
if(strcmp(req->path,'/index.html')==0){
...
}

Or another option is to have evhttp_parse_query_str() deal with the [path] part of a URI in an elegant way, I think we can all agree that turning the path into a key is a bug.

@nmathewson
Copy link
Member

New programs should not be using request->uri, or any of the other fields in request, directly; they should instead use the evhttp_request_get_* accessor functions.

That said, evhttp_request_get_uri is still a bad name. We should make sure that there exist functions that do the right thing, and we should make sure that the badness of the bad name.

evhttp_request_get_evhttp_uri is supposed to be the function that does the right thing (when combined with the evhttp_uri accessors). This does need getter documentation though.

Renaming evhttp_request_get_uri is not a live option: we try to NEVER introduce changes that will break existing programs that use the API correctly.

@TheRook
Copy link
Author

TheRook commented Feb 27, 2013

Thanks for the quick response, but your response doesn't address my bug. evhttp_request_get_uri() still returns a part of the path, and so evhttp_find_header's first key value is corrupt. This is the code i have to use to fix the [query] part of the URI:

query=evhttp_request_get_uri(req);
//Find the query part of the URI.
query=strstr(query,"?")+1;
if(query<=1){
    query=evhttp_request_get_uri(req);
}
evhttp_parse_query_str(query, &GET);
info_hash = (char *)evhttp_find_header(&GET, "key");

@nmathewson
Copy link
Member

To make sure I understand, is there a reason you can't just do the preferred approach, which is:

const struct evhttp_uri *uri = evhttp_request_get_evhttp_uri(req);
query = evhttp_uri_get_query(uri);
evhttp_parse_query_str(query, &GET);

?

@TheRook
Copy link
Author

TheRook commented Feb 28, 2013

Thanks, that works, but very unexpectedly. In general I find this interface to be unnatural. For example why do i use a find_header() function for non-http-header elements like GET and POST? Getting the user's IP address is also strange:

        evhttp_connection_get_peer(req->evcon,
                                   &client_ip,
                                   &scoket_client_port);

I think qdecoder has a nice and regular interface:
http://www.qdecoder.org/wiki/qdecoder

The difference is that the request struct is the mother argument for all accessor functions. Ideally you would just have to free the request struct, i.e. evhttp_request_free(req), at it would appropriately tear down all of the accessed data.

I like PHP's $_GET, $_POST, $_COOKIE, $_SERVER hash table system a lot. I should just be able to say:
char * key = evhttp_get_var(req, "key");
char * key = evhttp_post_var(req, "key");
char * proxy = evhttp_server_var(req, "x-forwarded-for");

@errzey
Copy link
Contributor

errzey commented Feb 28, 2013

I went ahead and ported libevhtp's query parser over to use evkeyvalq. You can cut & paste the code into yours here: https://gist.github.com/ellzey/5060433

@errzey
Copy link
Contributor

errzey commented Jun 6, 2015

Oh neat, I already responded to this. The above is old, you can check out libevhtp's version, it's actually faster now.

I'm closing this since we plan on merging evhtp with evhttp in the alpha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants