Message ID | 20201112221059.68098-1-martin@martin.st |
---|---|
State | Accepted |
Commit | 104e5233f03f1ed3aa34b256cd2b9023a9076ffb |
Headers | show |
Series | [FFmpeg-devel] http: Check for AVERROR_EOF in the check for premature end | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
On Fri, 13 Nov 2020, Martin Storsjö wrote: > When the check was added (in 3668701f9600, in 2015), some IO > functions returned 0 on EOF (in particular, the TCP protocol > did, but the TLS protocol returned AVERROR_EOF). Since > 0e1f771d2200d in 2017, the TCP protocol also returns AVERROR_EOF > instead of 0, making the check for premature end never have the > intended effect. > --- > libavformat/http.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/http.c b/libavformat/http.c > index 3d25d652d3..2d24c00e18 100644 > --- a/libavformat/http.c > +++ b/libavformat/http.c > @@ -1436,7 +1436,8 @@ static int http_buf_read(URLContext *h, uint8_t *buf, int size) > if ((!s->willclose || s->chunksize == UINT64_MAX) && s->off >= target_end) > return AVERROR_EOF; > len = ffurl_read(s->hd, buf, size); > - if (!len && (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { > + if ((!len || len == AVERROR_EOF) && > + (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { > av_log(h, AV_LOG_ERROR, > "Stream ends prematurely at %"PRIu64", should be %"PRIu64"\n", > s->off, target_end > -- > 2.24.3 (Apple Git-128) Ping? // Martin
On Fri, Nov 13, 2020 at 12:17 AM Martin Storsjö <martin@martin.st> wrote: > > When the check was added (in 3668701f9600, in 2015), some IO > functions returned 0 on EOF (in particular, the TCP protocol > did, but the TLS protocol returned AVERROR_EOF). Since > 0e1f771d2200d in 2017, the TCP protocol also returns AVERROR_EOF > instead of 0, making the check for premature end never have the > intended effect. > --- > libavformat/http.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/http.c b/libavformat/http.c > index 3d25d652d3..2d24c00e18 100644 > --- a/libavformat/http.c > +++ b/libavformat/http.c > @@ -1436,7 +1436,8 @@ static int http_buf_read(URLContext *h, uint8_t *buf, int size) > if ((!s->willclose || s->chunksize == UINT64_MAX) && s->off >= target_end) > return AVERROR_EOF; > len = ffurl_read(s->hd, buf, size); > - if (!len && (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { > + if ((!len || len == AVERROR_EOF) && > + (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { > av_log(h, AV_LOG_ERROR, > "Stream ends prematurely at %"PRIu64", should be %"PRIu64"\n", > s->off, target_end > -- > 2.24.3 (Apple Git-128) Change-wise should be OK. Empty or EOF with not enough read leading to AVERROR(EIO), former being already current behavior. That said, if this error case always was only regarding EOF use cases we might want to check the case in ticket 4039 - if simplifying the check to EOF only still has it work as expected. But that I think is a separate check/change. Jan
On Fri, 20 Nov 2020, Jan Ekström wrote: > On Fri, Nov 13, 2020 at 12:17 AM Martin Storsjö <martin@martin.st> wrote: >> >> When the check was added (in 3668701f9600, in 2015), some IO >> functions returned 0 on EOF (in particular, the TCP protocol >> did, but the TLS protocol returned AVERROR_EOF). Since >> 0e1f771d2200d in 2017, the TCP protocol also returns AVERROR_EOF >> instead of 0, making the check for premature end never have the >> intended effect. >> --- >> libavformat/http.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/http.c b/libavformat/http.c >> index 3d25d652d3..2d24c00e18 100644 >> --- a/libavformat/http.c >> +++ b/libavformat/http.c >> @@ -1436,7 +1436,8 @@ static int http_buf_read(URLContext *h, uint8_t *buf, int size) >> if ((!s->willclose || s->chunksize == UINT64_MAX) && s->off >= target_end) >> return AVERROR_EOF; >> len = ffurl_read(s->hd, buf, size); >> - if (!len && (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { >> + if ((!len || len == AVERROR_EOF) && >> + (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { >> av_log(h, AV_LOG_ERROR, >> "Stream ends prematurely at %"PRIu64", should be %"PRIu64"\n", >> s->off, target_end >> -- >> 2.24.3 (Apple Git-128) > > Change-wise should be OK. Empty or EOF with not enough read leading to > AVERROR(EIO), former being already current behavior. > > That said, if this error case always was only regarding EOF use cases > we might want to check the case in ticket 4039 - if simplifying the > check to EOF only still has it work as expected. But that I think is a > separate check/change. Thanks for having a look, will push this one soon then. Yeah, removing the check for !len might be fine when/if FF_API_OLD_AVIO_EOF_0 gets disabled. // Martin
diff --git a/libavformat/http.c b/libavformat/http.c index 3d25d652d3..2d24c00e18 100644 --- a/libavformat/http.c +++ b/libavformat/http.c @@ -1436,7 +1436,8 @@ static int http_buf_read(URLContext *h, uint8_t *buf, int size) if ((!s->willclose || s->chunksize == UINT64_MAX) && s->off >= target_end) return AVERROR_EOF; len = ffurl_read(s->hd, buf, size); - if (!len && (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { + if ((!len || len == AVERROR_EOF) && + (!s->willclose || s->chunksize == UINT64_MAX) && s->off < target_end) { av_log(h, AV_LOG_ERROR, "Stream ends prematurely at %"PRIu64", should be %"PRIu64"\n", s->off, target_end