[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [Libevent-users] bug + question



On Thu, Dec 30, 2010 at 8:21 PM, Mihai Draghicioiu
<mihai.draghicioiu@xxxxxxxxx> wrote:
> Hi all. I've found that evhttp_parse_query_str only works if the query
> string is of the form foo=bar&baz=quux. If we have a query string of
> the form foo=bar&baz (no value for the last key), it fails. I consider
> this a bug, because you have all the other valid parameters in there,
> and it can just set NULL for the value of the baz parameter. I believe
> the behavior of evhttp_parse_query() is different (correct). I had
> used evhttp_parse_query() until now, and switched to
> evhttp_parse_query_str() and discovered this bug.

I'll repost my question from
https://sourceforge.net/tracker/?func=detail&aid=3037662&group_id=50884&atid=461322
:

===
So, RFC3986 says that the query part of a URL can be any sequence of
characters in a given set, without providing any instructions on how to
encode or decode them.

The only place that i can find the key=val&key2=val2 format expressed, on
the other hand, is in the HTML standard, which doesn't define anything
about parameters without values.

So I'm inclined to say that this is not-a-bug: If the query parameters are
not encoded in the HTML-standard way, then absent some other standard, then
decoding the Request-URI from the Request-Line is up to the library user,
right?

Is there a standard I'm missing here?
===

In other words, according to what standard is foo=bar&baz to be
interpreted?  If we're considering generic URIs, there is no
particular interpretation to the query part.  If we're considering GET
requests generated by HTML forms, I think it's invalid, if I'm reading
the HTML spec right.  So according to what standard does foo=bar&baz
have an interpretation?

> Second, I have a question - does one need to evhttp_uri_free() the
> returned pointer from evhttp_request_get_evhttp_uri()? It crashes if I
> do free it, so i'd like to make sure it's correct to not free it (no
> memory leaks).

Right.  (In fact, you shouldn't be able to write correct code that
tries to free it!  The evhttp_request_get_evhttp_uri() function
returns a const pointer, and evhttp_uri_free() requires a non-const
pointer.)

> Also, is the evkeyvalq usage in this example correct? Note that it
> includes the issues described above:
>
> static void httpcb(struct evhttp_request *req, void *arg) {
>    evkeyvalq query; // is it meant to be used in this way?
>    evhttp_parse_query_str(evhttp_uri_get_query(evhttp_request_get_evhttp_uri(req)),
> &query);

Hm.  This is a little iffy, since evhttp_uri_get_query can return
NULL, and evhttp_parse_query_str is not documented to accept NULL.
(It can and does, but I'd still call usage of an undocumented feature
questionable.)

Also, you aren't checking the return value of evhttp_parse_query_str.

>    const char *val;
>    if((val = evhttp_find_header(&query, "command"))) {

Looks fine to me.

>        ...
>    }
> }


yrs,
-- 
Nick
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.