You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2018/08/18 01:19:00 UTC
Review Request 68422: Added regression test for
`http::internal::encode(Request)`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68422/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Joseph Wu.
Repository: mesos
Description
-------
This function is supposed to be encoding the query parameters, but
currently fails to do so, resulting in:
```
libprocess\src\tests\http_tests.cpp(655): error: Value of: read.get()
Expected: starts with "GET /?path=C%3A%5Cfoo%5Cbar%253Abaz"
Actual: "GET /?path=C:\foo\bar%3Abaz HTTP/1.1\r\nConnection:
close\r\nHost: mesos.apache.org\r\nContent-Length: 0\r\n\r\n"
```
The query parameter is going over the wire unencoded.
Diffs
-----
3rdparty/libprocess/src/tests/http_tests.cpp 89d3bb2f91efe97591fe3f43e947413cf8b61f8c
Diff: https://reviews.apache.org/r/68422/diff/1/
Testing
-------
Thanks,
Andrew Schwartzmeyer
Re: Review Request 68422: Added regression test for
`http::internal::encode(Request)`.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68422/#review207771
-----------------------------------------------------------
Ship it!
Thanks Andy!
3rdparty/libprocess/src/tests/http_tests.cpp
Lines 112-113 (patched)
<https://reviews.apache.org/r/68422/#comment291276>
// TODO(bmahler): The client's encoding logic is currently not
// exposed in headers, we declare it here to test it. This
// should be exposed in headers as a library.
3rdparty/libprocess/src/tests/http_tests.cpp
Lines 639 (patched)
<https://reviews.apache.org/r/68422/#comment291277>
Let's call this `QueryEncoding`, since we have to also test path, fragment, etc getting encoded.
- Benjamin Mahler
On Aug. 18, 2018, 1:19 a.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68422/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2018, 1:19 a.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Joseph Wu.
>
>
> Bugs: MESOS-9168
> https://issues.apache.org/jira/browse/MESOS-9168
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This function is supposed to be encoding the query parameters, but
> currently fails to do so, resulting in:
>
> ```
> libprocess\src\tests\http_tests.cpp(655): error: Value of: read.get()
> Expected: starts with "GET /?path=C%3A%5Cfoo%5Cbar%253Abaz"
> Actual: "GET /?path=C:\foo\bar%3Abaz HTTP/1.1\r\nConnection:
> close\r\nHost: mesos.apache.org\r\nContent-Length: 0\r\n\r\n"
> ```
>
> The query parameter is going over the wire unencoded.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/tests/http_tests.cpp 89d3bb2f91efe97591fe3f43e947413cf8b61f8c
>
>
> Diff: https://reviews.apache.org/r/68422/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 68422: Added regression test for
`http::internal::encode(Request)`.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68422/
-----------------------------------------------------------
(Updated Aug. 23, 2018, 11:26 a.m.)
Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Joseph Wu.
Changes
-------
Updated per comments.
Bugs: MESOS-9168
https://issues.apache.org/jira/browse/MESOS-9168
Repository: mesos
Description
-------
This function is supposed to be encoding the query parameters, but
currently fails to do so, resulting in:
```
libprocess\src\tests\http_tests.cpp(655): error: Value of: read.get()
Expected: starts with "GET /?path=C%3A%5Cfoo%5Cbar%253Abaz"
Actual: "GET /?path=C:\foo\bar%3Abaz HTTP/1.1\r\nConnection:
close\r\nHost: mesos.apache.org\r\nContent-Length: 0\r\n\r\n"
```
The query parameter is going over the wire unencoded.
Diffs (updated)
-----
3rdparty/libprocess/src/tests/http_tests.cpp 89d3bb2f91efe97591fe3f43e947413cf8b61f8c
Diff: https://reviews.apache.org/r/68422/diff/2/
Changes: https://reviews.apache.org/r/68422/diff/1-2/
Testing
-------
Thanks,
Andrew Schwartzmeyer