mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joseph Wu" <jos...@mesosphere.io>
Subject Re: Review Request 39611: Remove binary `data` fields from state endpoints.
Date Thu, 05 Nov 2015 00:04:23 GMT


> On Nov. 4, 2015, 2:59 p.m., Ben Mahler wrote:
> > docs/upgrades.md, line 16
> > <https://reviews.apache.org/r/39611/diff/2/?file=1114331#file1114331line16>
> >
> >     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.

I added onto the message.

I'm not sure how understandable the JSON "search string" is.  (Modelled after the syntax of
`JSON::Object::find`.)


- Joseph


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


On Nov. 4, 2015, 4:02 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39611/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 4:02 p.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