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/11/14 23:16:17 UTC
Review Request: Encoding JSON strings as ASCII (fix for binary data in JSON
strings).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
Diffs
-----
src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6
third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
Diff: https://reviews.apache.org/r/8056/diff/
Testing
-------
make check + added json test
Thanks,
Ben Mahler
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Ben Mahler <be...@gmail.com>.
> On Nov. 15, 2012, 9:58 p.m., Vinod Kone wrote:
> >
thanks!
> On Nov. 15, 2012, 9:58 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/stout/json.hpp, line 120
> > <https://reviews.apache.org/r/8056/diff/1/?file=189961#file189961line120>
> >
> > s/are also escaping/also escape/
> >
> > s/we're not encode/ we don't escape/
fixed
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/#review13488
-----------------------------------------------------------
On Nov. 14, 2012, 10:16 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8056/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2012, 10:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
>
> This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
>
>
> Diffs
> -----
>
> src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
> third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6
> third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
>
> Diff: https://reviews.apache.org/r/8056/diff/
>
>
> Testing
> -------
>
> make check + added json test
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/#review13488
-----------------------------------------------------------
Ship it!
third_party/libprocess/include/stout/json.hpp
<https://reviews.apache.org/r/8056/#comment28925>
s/are also escaping/also escape/
s/we're not encode/ we don't escape/
- Vinod Kone
On Nov. 14, 2012, 10:16 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8056/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2012, 10:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
>
> This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
>
>
> Diffs
> -----
>
> src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
> third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6
> third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
>
> Diff: https://reviews.apache.org/r/8056/diff/
>
>
> Testing
> -------
>
> make check + added json test
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/#review14312
-----------------------------------------------------------
third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/8056/#comment30471>
Let's kill the casts since they aren't needed and their presence makes me think that they are needed.
third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/8056/#comment30472>
Ditto.
third_party/libprocess/include/stout/json.hpp
<https://reviews.apache.org/r/8056/#comment30470>
I think it would be great to have a small comment here explaining that the spec requires four numbers thus we need to 'setw' and also cast to 'unsigned int'.
- Benjamin Hindman
On Nov. 15, 2012, 10:01 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8056/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2012, 10:01 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
>
> This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
>
>
> Diffs
> -----
>
> src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
> third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6
> third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
>
> Diff: https://reviews.apache.org/r/8056/diff/
>
>
> Testing
> -------
>
> make check + added json test
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/#review14761
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On Dec. 19, 2012, 7:02 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8056/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2012, 7:02 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
>
> This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
>
>
> This addresses bug MESOS-274.
> https://issues.apache.org/jira/browse/MESOS-274
>
>
> Diffs
> -----
>
> src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
> third_party/libprocess/include/process/http.hpp ced8c1ab69952c4297d92b472f8d3edb5954105c
> third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
> third_party/libprocess/src/tests.cpp 131154abc920fa3904b7fa79faedb63f8d593793
>
> Diff: https://reviews.apache.org/r/8056/diff/
>
>
> Testing
> -------
>
> make check + added json test
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/
-----------------------------------------------------------
(Updated Dec. 19, 2012, 7:02 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
This addresses bug MESOS-274.
https://issues.apache.org/jira/browse/MESOS-274
Diffs
-----
src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
third_party/libprocess/include/process/http.hpp ced8c1ab69952c4297d92b472f8d3edb5954105c
third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
third_party/libprocess/src/tests.cpp 131154abc920fa3904b7fa79faedb63f8d593793
Diff: https://reviews.apache.org/r/8056/diff/
Testing
-------
make check + added json test
Thanks,
Ben Mahler
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/
-----------------------------------------------------------
(Updated Dec. 19, 2012, 7:02 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Fixed http encode.
Now both need the cast to unsigned int, so they look symmetric.
Description
-------
At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
Diffs (updated)
-----
src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
third_party/libprocess/include/process/http.hpp ced8c1ab69952c4297d92b472f8d3edb5954105c
third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
third_party/libprocess/src/tests.cpp 131154abc920fa3904b7fa79faedb63f8d593793
Diff: https://reviews.apache.org/r/8056/diff/
Testing
-------
make check + added json test
Thanks,
Ben Mahler
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/
-----------------------------------------------------------
(Updated Dec. 12, 2012, 6:32 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Small changes from benh review.
Description
-------
At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
Diffs (updated)
-----
src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
third_party/libprocess/include/process/http.hpp ced8c1ab69952c4297d92b472f8d3edb5954105c
third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
Diff: https://reviews.apache.org/r/8056/diff/
Testing
-------
make check + added json test
Thanks,
Ben Mahler
Re: Review Request: Encoding JSON strings as ASCII (fix for binary data in
JSON strings).
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8056/
-----------------------------------------------------------
(Updated Nov. 15, 2012, 10:01 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
At twitter, we had an executor using binary data in the TaskInfo.data field. Since we can't encode unicode / binary data, this breaks the JSON endpoint and the webui.
This fixes the break, as well as allows browsing binary files in the webui (however, encoded as ASCII).
Diffs (updated)
-----
src/tests/stout_tests.cpp efdb47f1d2401f9175a05fc65922d9ab7e383666
third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6
third_party/libprocess/include/stout/json.hpp 1c66dfd324619222f4ed168b7cf18e8f5e558b23
Diff: https://reviews.apache.org/r/8056/diff/
Testing
-------
make check + added json test
Thanks,
Ben Mahler