mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 68420: Added /files API test for reserved query characters.
Date Tue, 04 Sep 2018 17:28:02 GMT


> On Aug. 23, 2018, 1:02 p.m., Benjamin Mahler wrote:
> > src/tests/files_tests.cpp
> > Lines 331-332 (patched)
> > <https://reviews.apache.org/r/68420/diff/1/?file=2074999#file2074999line331>
> >
> >     This is a little odd to read without the context, this tests reserved characters
by using `%`, but we could use anything other reserved character to acheive the same test,
e.g. `+` seems the simplest:
> >     
> >     ```
> >       // We use a reserved character for query parameters `+` to
> >       // ensure it is encoded and decoded correctly.
> >       const string filename = "foo+bar";
> >     ```
> >     
> >     If you'd like to stick with using `%`, probably we should just say:
> >     
> >     ```
> >       // We use a reserved character for query parameters `%` to
> >       // ensure it is encoded and decoded correctly.
> >       const string filename = "foo%3Abar";
> >     ```
> >     
> >     And not do line 334 as it's irrelevant to this test
> 
> Andrew Schwartzmeyer wrote:
>     You couldn't use `+` because it wouldn't have the same problem: the second decode
would not turn `+` into something else (this is why we haven't run into this problem before
with other characters, even a literal `:`). The problem is that `%3A` _can_ be decoded, and
so decoding more times than encoding causes it to erroneously become `:`. The number of encodes
and decodes must be balanced.
>     
>     Also, line 334 is relevant to this test: it's demonstrating that the query can be
erroneously decoded too many times.
> 
> Andrew Schwartzmeyer wrote:
>     I explained why thits is explicitly an entire percent-encoding, and am considering
this fixed. Thanks!

Okay I am going to assume this was "fix it, then ship it" as you marked the other two as ship
it, and I'd like to get at least the tests and libprocess fix in.


- Andrew


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


On Aug. 28, 2018, 10:40 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68420/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2018, 10:40 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9168
>     https://issues.apache.org/jira/browse/MESOS-9168
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Due to a bug in the HTTP client in libprocess (MESOS-9168), the
> use of reserved query characters in paths did not work when using
> the HTTP client in libprocess. This adds a test to ensure that
> the /files API correctly handles reserved query characters in
> file paths.
> 
> 
> Diffs
> -----
> 
>   src/tests/files_tests.cpp d09279077ec704167991933349ee943ab44d0aa9 
> 
> 
> Diff: https://reviews.apache.org/r/68420/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message