You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2017/02/22 00:20:04 UTC
Review Request 56906: WIP: Dealt with a corner case in the ReadFile
API.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56906/
-----------------------------------------------------------
Review request for mesos and Anand Mazumdar.
Repository: mesos
Description
-------
The canonical integer type used when dealing with files in Posix
is an `off_t`, or a signed 64-bit integer. In our `FilesProcess`,
we actually use a `size_t`, an unsigned 64-bit integer, to represent
file offsets and lengths. This opens up the potential for passing in
negative values (in 2's complement, an unsigned value greater than
2^63 - 1 becomes a negative value when casted to a signed value)
for the offset and length.
In the current code, this leads to a discrepancy in return values
when a caller uses the `/files/read` endpoint versus the `ReadFile`
V1 API:
Calling `/files/read?offset=9223372036854775808` (2^63) will return a
`400 Bad Request`. Performing the equivalent call via `ReadFile`
will instead return a `200 OK`, with no data actually read.
Diffs
-----
src/files/files.hpp 8cffc26fc7d674187e55663f23f1e10bed40229e
src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a
src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1
src/slave/http.cpp af70b6f294a04f23b04cd1d8c36c1c3e86d7d5e6
src/tests/api_tests.cpp 378612dd4d038fb4d65fba60a4be00d4950d0c02
src/tests/files_tests.cpp 6c6353e406249f021803e83909415e9908ded28c
Diff: https://reviews.apache.org/r/56906/diff/
Testing
-------
make check
src/mesos-tests --gtest_filter="*FilesTest*:*MasterAPITest*"
---
NOTE: The new code added to `api_tests.cpp` will actually fail for `application/json`, but succeed with protobuf:
ContentType/MasterAPITest.ReadFile/1, where GetParam() = application/json
Thanks,
Joseph Wu
Re: Review Request 56906: Dealt with a corner case in the ReadFile
API.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56906/#review166996
-----------------------------------------------------------
Patch looks great!
Reviews applied: [56906]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Feb. 28, 2017, 12:46 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56906/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2017, 12:46 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The canonical integer type used when dealing with files in Posix
> is an `off_t`, or a signed 64-bit integer. In our `FilesProcess`,
> we actually use a `size_t`, an unsigned 64-bit integer, to represent
> file offsets and lengths. This opens up the potential for passing in
> negative values (in 2's complement, an unsigned value greater than
> 2^63 - 1 becomes a negative value when casted to a signed value)
> for the offset and length.
>
> In the current code, this leads to a discrepancy in return values
> when a caller uses the `/files/read` endpoint versus the `ReadFile`
> V1 API:
>
> Calling `/files/read?offset=9223372036854775808` (2^63) will return a
> `400 Bad Request`. Performing the equivalent call via `ReadFile`
> will instead return a `200 OK`, with no data actually read.
>
>
> Diffs
> -----
>
> src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a
> src/tests/api_tests.cpp 24f31ff133185a3224f9edaf8b8532d5630b34c2
> src/tests/files_tests.cpp 6c6353e406249f021803e83909415e9908ded28c
>
> Diff: https://reviews.apache.org/r/56906/diff/
>
>
> Testing
> -------
>
> make check
>
> src/mesos-tests --gtest_filter="*FilesTest*:*MasterAPITest*"
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 56906: Dealt with a corner case in the ReadFile
API.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56906/
-----------------------------------------------------------
(Updated Feb. 27, 2017, 4:46 p.m.)
Review request for mesos and Anand Mazumdar.
Changes
-------
Changed to a less-invasive approach, of just changing one continuation of the `FilesProcess` with some extra validation logic. This results in a much smaller patch overall, and less duplication.
Summary (updated)
-----------------
Dealt with a corner case in the ReadFile API.
Repository: mesos
Description
-------
The canonical integer type used when dealing with files in Posix
is an `off_t`, or a signed 64-bit integer. In our `FilesProcess`,
we actually use a `size_t`, an unsigned 64-bit integer, to represent
file offsets and lengths. This opens up the potential for passing in
negative values (in 2's complement, an unsigned value greater than
2^63 - 1 becomes a negative value when casted to a signed value)
for the offset and length.
In the current code, this leads to a discrepancy in return values
when a caller uses the `/files/read` endpoint versus the `ReadFile`
V1 API:
Calling `/files/read?offset=9223372036854775808` (2^63) will return a
`400 Bad Request`. Performing the equivalent call via `ReadFile`
will instead return a `200 OK`, with no data actually read.
Diffs (updated)
-----
src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a
src/tests/api_tests.cpp 24f31ff133185a3224f9edaf8b8532d5630b34c2
src/tests/files_tests.cpp 6c6353e406249f021803e83909415e9908ded28c
Diff: https://reviews.apache.org/r/56906/diff/
Testing (updated)
-------
make check
src/mesos-tests --gtest_filter="*FilesTest*:*MasterAPITest*"
Thanks,
Joseph Wu