> On Nov. 20, 2014, 9:11 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 415-416
> > <https://reviews.apache.org/r/28253/diff/1/?file=770208#file770208line415>
> >
> >     Here's what we'll have if we added join:
> >     
> >     parse: takes a __decoded__ query string, construct a key,value map of 
> > the parameters
> >     
> >     join: takes a key,value map of parameters, creates an __encoded__ query 
> > string.
> >     
> >     That's where I find this to be non-intuitive:
> >     
> >     ```
> >     join(parse(query)) == encode(query)
> >     ```
> >     
> >     It seems like only request encoding should deal with constructing an 
> > encoded query string, is that not the case? Would like to avoid exposing 
> > this if possible, and if not possible, would like to make this more 
> > intuitive.

So that behavior of parse is going to produce incorrect results I believe. 
Suppose one of my parameters in my query string is a small blob of JSON 
(Contains '=' and '&' potentially). If we encode/decode on the outside of query 
string parsing/joining, we can't represent this (The & will look like the start 
of a new parameter).

```
std::map<> query = {{"a%20","b&c=d!@#foobar"}}

EXPECT_EQ(query, parse(join(query)));
```

Based on my reading of rfc3986 2.1 (snippet inline):
```
A percent-encoding mechanism is used to represent a data octet in a
component when that octet's corresponding character is outside the
allowed set or is being used as a delimiter of, or within, the
component.
```

This should work, and is the reason why there is percent encoding. From what I 
can tell grepping throuhg the source code nothing currently does the decoding 
before we parse the query string, so making the decode live inside of the parse 
function should work. Currently callers just end up with percent encoded chunks 
in their query string, which causes badness (And could explain an issue which a 
customer was having with files/browse.json and things not showing up)


- Cody


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28253/#review62422
-----------------------------------------------------------


On Nov. 20, 2014, 12:50 a.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28253/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2014, 12:50 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2132
>     https://issues.apache.org/jira/browse/MESOS-2132
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This is needed to turn Request objects into http request which we send across 
> the wire. The existing HTTP request sending code expects the query string to 
> be already constructed.
> 
> Ideally people can either just forward the query string they get to send it 
> back across the wire or they can manipulate it using the native C++ datatype 
> (A hashmap), and then have it automatically re-encoded.
> 
> The parsing logic to go from a string to a map is already exists in this 
> header, this just adds the other direction.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 9cf05acbb724ab9af8010d1788621d37a0e48e86 
>   3rdparty/libprocess/src/http.cpp b00f33339366f5c06b6f20e38c5ae0c23b8a9358 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> a90e65f77904da0a45e1cc0cc9889ae69354a1a5 
> 
> Diff: https://reviews.apache.org/r/28253/diff/
> 
> 
> Testing
> -------
> 
> make distcheck ubuntu 14.04
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Reply via email to