mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 39611: Remove binary `data` fields from state endpoints.
Date Wed, 04 Nov 2015 22:59:56 GMT

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

Ship it!


Thanks Joseph, just a minor comment about the upgrade note.


docs/upgrades.md (line 16)
<https://reviews.apache.org/r/39611/#comment163589>

    Can we be clear about exactly what is being removed here and give some background (link
to the ticket)? Specifically, it would be helpful to say exactly which fields instead of referring
to arbitrary binary data.


- Ben Mahler


On Nov. 3, 2015, 1:06 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39611/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2015, 1:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3771
>     https://issues.apache.org/jira/browse/MESOS-3771
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We do not encode or otherwise preprocess the raw binary in the ExecutorInfo or TaskInfo
`data` fields before outputting them to JSON via the state endpoints.  This means that the
outputted JSON may be invalid (i.e. non-UTF8) if `data` was filled in with arbitrary bytes.
> 
> The `data` fields are being removed because the state endpoints should not be writing
binary data in the first place.
> 
> 
> Diffs
> -----
> 
>   docs/upgrades.md 3854e59f68d53892b2673a4542ea2aaf96936cbb 
>   src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
>   src/slave/http.cpp d6df97fb64077fb051dd78fe35bf04e8be0331ff 
> 
> Diff: https://reviews.apache.org/r/39611/diff/
> 
> 
> Testing
> -------
> 
> `make check` and visually checked markdown for correctness.
> 
> Apparently, none of our tests check these fields.  
> 
> (Discovered via manual tests) Our JSON parser (PicoJson) is not restrictive enough to
catch the invalid-JSON that results.  It will happily parse the binary blobs (roundtripping
successfully).
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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