You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2012/09/26 01:01:29 UTC

Review Request: HTTP Percent Encoding / Decoding.

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

Review request for mesos, Benjamin Hindman and Vinod Kone.


Description
-------

We currently don't decode percent-encoded queries!


Diffs
-----

  src/tests/files_tests.cpp 17d284b82de4d9e1dedf1c26f4cedcf8269c4072 
  third_party/libprocess/include/process/http.hpp d680261f9a460627e0ea8c492b38c6dac83f3b56 
  third_party/libprocess/src/decoder.hpp f582faf985a17b1b333331da3582caa20b806ec0 
  third_party/libprocess/src/tests.cpp 1df7ae61f24643a0ceab146646e77da6909dfd5f 

Diff: https://reviews.apache.org/r/7272/diff/


Testing
-------

Tests for encoding / decoding.


Thanks,

Ben Mahler


Re: Review Request: HTTP Percent Encoding / Decoding.

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 8, 2012, 10:10 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/http.hpp, line 255
> > <https://reviews.apache.org/r/7272/diff/2/?file=160814#file160814line255>
> >
> >     std::istringstream in(s.substr(i + 1, 2));
> >     long l;
> >     in >> std::hex >> l;
> >     CHECK(... 'l' fits in a char ...);
> >     out << static_cast<char>(l);
> >

Changed, but it seems uglier, although probably more readable..


- Ben


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


On Sept. 28, 2012, 9:21 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7272/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2012, 9:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> We currently don't decode percent-encoded queries!
> 
> 
> Diffs
> -----
> 
>   src/tests/files_tests.cpp 17d284b 
>   third_party/libprocess/include/process/http.hpp d680261 
>   third_party/libprocess/src/decoder.hpp f582faf 
>   third_party/libprocess/src/tests.cpp 1df7ae6 
> 
> Diff: https://reviews.apache.org/r/7272/diff/
> 
> 
> Testing
> -------
> 
> Tests for encoding / decoding.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: HTTP Percent Encoding / Decoding.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7272/#review12246
-----------------------------------------------------------

Ship it!



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7272/#comment25986>

    s/encodeURI/encode/



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7272/#comment25987>

    s/decodeURI/decode/



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7272/#comment25988>

    std::istringstream in(s.substr(i + 1, 2));
    long l;
    in >> std::hex >> l;
    CHECK(... 'l' fits in a char ...);
    out << static_cast<char>(l);
    


- Benjamin Hindman


On Sept. 28, 2012, 9:21 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7272/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2012, 9:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> We currently don't decode percent-encoded queries!
> 
> 
> Diffs
> -----
> 
>   src/tests/files_tests.cpp 17d284b 
>   third_party/libprocess/include/process/http.hpp d680261 
>   third_party/libprocess/src/decoder.hpp f582faf 
>   third_party/libprocess/src/tests.cpp 1df7ae6 
> 
> Diff: https://reviews.apache.org/r/7272/diff/
> 
> 
> Testing
> -------
> 
> Tests for encoding / decoding.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: HTTP Percent Encoding / Decoding.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7272/
-----------------------------------------------------------

(Updated Oct. 9, 2012, 12:35 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Rebased off trunk.
Updated with benh's comments.


Description
-------

We currently don't decode percent-encoded queries!


Diffs (updated)
-----

  src/tests/files_tests.cpp 17d284b82de4d9e1dedf1c26f4cedcf8269c4072 
  third_party/libprocess/include/process/http.hpp d680261f9a460627e0ea8c492b38c6dac83f3b56 
  third_party/libprocess/src/decoder.hpp f582faf985a17b1b333331da3582caa20b806ec0 
  third_party/libprocess/src/tests.cpp 1df7ae61f24643a0ceab146646e77da6909dfd5f 

Diff: https://reviews.apache.org/r/7272/diff/


Testing
-------

Tests for encoding / decoding.


Thanks,

Ben Mahler


Re: Review Request: HTTP Percent Encoding / Decoding.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7272/
-----------------------------------------------------------

(Updated Sept. 28, 2012, 9:21 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Description
-------

We currently don't decode percent-encoded queries!


Diffs (updated)
-----

  src/tests/files_tests.cpp 17d284b 
  third_party/libprocess/include/process/http.hpp d680261 
  third_party/libprocess/src/decoder.hpp f582faf 
  third_party/libprocess/src/tests.cpp 1df7ae6 

Diff: https://reviews.apache.org/r/7272/diff/


Testing
-------

Tests for encoding / decoding.


Thanks,

Ben Mahler


Re: Review Request: HTTP Percent Encoding / Decoding.

Posted by Ben Mahler <be...@gmail.com>.

> On Sept. 28, 2012, 7:20 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/http.hpp, line 239
> > <https://reviews.apache.org/r/7272/diff/1/?file=160108#file160108line239>
> >
> >     Rephrase?
> >     
> >     Returns error on the occurence of a malformed % in s.

done.


> On Sept. 28, 2012, 7:20 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/http.hpp, line 186
> > <https://reviews.apache.org/r/7272/diff/1/?file=160108#file160108line186>
> >
> >     s/encodeURIComponent/encodeURI ?

Just FYI I borrowed the names of these functions from the javascript builtins: encodeURIComponent / decodeURIComponent.

Changed to encodeURI / decodeURI (however, note that the input string does not have to be a URI, but can be a component of a URI).


> On Sept. 28, 2012, 7:20 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/http.hpp, line 252
> > <https://reviews.apache.org/r/7272/diff/1/?file=160108#file160108line252>
> >
> >     probably useful to also include the position 'i' in the error string.

Done, but instead I decided to return the malformed bit of the string.


- Ben


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


On Sept. 25, 2012, 11:01 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7272/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2012, 11:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> We currently don't decode percent-encoded queries!
> 
> 
> Diffs
> -----
> 
>   src/tests/files_tests.cpp 17d284b82de4d9e1dedf1c26f4cedcf8269c4072 
>   third_party/libprocess/include/process/http.hpp d680261f9a460627e0ea8c492b38c6dac83f3b56 
>   third_party/libprocess/src/decoder.hpp f582faf985a17b1b333331da3582caa20b806ec0 
>   third_party/libprocess/src/tests.cpp 1df7ae61f24643a0ceab146646e77da6909dfd5f 
> 
> Diff: https://reviews.apache.org/r/7272/diff/
> 
> 
> Testing
> -------
> 
> Tests for encoding / decoding.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: HTTP Percent Encoding / Decoding.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7272/#review12030
-----------------------------------------------------------



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7272/#comment25649>

    s/encodeURIComponent/encodeURI ?



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7272/#comment25647>

    Rephrase?
    
    Returns error on the occurence of a malformed % in s.



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7272/#comment25650>

    s/decodeURIComponent/decodeURI/ ?



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7272/#comment25648>

    probably useful to also include the position 'i' in the error string.


- Vinod Kone


On Sept. 25, 2012, 11:01 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7272/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2012, 11:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> We currently don't decode percent-encoded queries!
> 
> 
> Diffs
> -----
> 
>   src/tests/files_tests.cpp 17d284b82de4d9e1dedf1c26f4cedcf8269c4072 
>   third_party/libprocess/include/process/http.hpp d680261f9a460627e0ea8c492b38c6dac83f3b56 
>   third_party/libprocess/src/decoder.hpp f582faf985a17b1b333331da3582caa20b806ec0 
>   third_party/libprocess/src/tests.cpp 1df7ae61f24643a0ceab146646e77da6909dfd5f 
> 
> Diff: https://reviews.apache.org/r/7272/diff/
> 
> 
> Testing
> -------
> 
> Tests for encoding / decoding.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>