From 6db619c220ddc17ae5704d7235fa206386c65d20 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 14 Feb 2024 14:08:49 -0500 Subject: [PATCH 1/5] Update readbytes! to avoid using internal API of IOBuffer less Note that this claims to be supported for all GenericIOBuffer, but Base only defines this function on a normal IOBuffer for which `pointer(data)` is contiguous space --- src/Streams.jl | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/Streams.jl b/src/Streams.jl index 220d11c4e..d44c64450 100644 --- a/src/Streams.jl +++ b/src/Streams.jl @@ -279,17 +279,10 @@ function Base.unsafe_read(http::Stream, p::Ptr{UInt8}, n::UInt) nothing end -@noinline function bufcheck(buf::Base.GenericIOBuffer, n) - requested_buffer_capacity = (buf.append ? buf.size : (buf.ptr - 1)) + n - requested_buffer_capacity > length(buf.data) && throw(ArgumentError("Unable to grow response stream IOBuffer $(length(buf.data)) large enough for response body size: $requested_buffer_capacity")) -end - function Base.readbytes!(http::Stream, buf::Base.GenericIOBuffer, n=bytesavailable(http)) - Base.ensureroom(buf, n) - # check if there's enough room in buf to write n bytes - bufcheck(buf, n) - data = buf.data - GC.@preserve data unsafe_read(http, pointer(data, (buf.append ? buf.size + 1 : buf.ptr)), n) + p, nbmax = Base.alloc_request(buf, n) + n = Int(GC.@preserve buf unsafe_read(http, p, min(nbmax, n))) + # TODO: use `Base.notify_filled(buf, n)` here, but only once it is identical to this: if buf.append buf.size += n else @@ -326,7 +319,8 @@ function IOExtras.readuntil(http::Stream, f::Function)::ByteView bytes = IOExtras.readuntil(http.stream, f) update_ntoread(http, length(bytes)) return bytes - catch + catch ex + ex isa EOFError() || rethrow() # if we error, it means we didn't find what we were looking for return UInt8[] end From e83cc6db307d3fa7f1dfdbd8078ced00c2492551 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 16 Feb 2024 20:05:43 -0500 Subject: [PATCH 2/5] Update Streams.jl --- src/Streams.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Streams.jl b/src/Streams.jl index d44c64450..2f50b4eec 100644 --- a/src/Streams.jl +++ b/src/Streams.jl @@ -280,8 +280,8 @@ function Base.unsafe_read(http::Stream, p::Ptr{UInt8}, n::UInt) end function Base.readbytes!(http::Stream, buf::Base.GenericIOBuffer, n=bytesavailable(http)) - p, nbmax = Base.alloc_request(buf, n) - n = Int(GC.@preserve buf unsafe_read(http, p, min(nbmax, n))) + p, nbmax = Base.alloc_request(buf, UInt(n)) + n = Int(GC.@preserve buf unsafe_read(http, p, UInt(min(nbmax, n)))) # TODO: use `Base.notify_filled(buf, n)` here, but only once it is identical to this: if buf.append buf.size += n From 0ca7e3fe5ce917684d335a5c513c3fd5368a7fbc Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 16 Feb 2024 20:21:38 -0500 Subject: [PATCH 3/5] Update Streams.jl --- src/Streams.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Streams.jl b/src/Streams.jl index ba7e30c8d..b54cadbc2 100644 --- a/src/Streams.jl +++ b/src/Streams.jl @@ -281,12 +281,12 @@ end function Base.readbytes!(http::Stream, buf::Base.GenericIOBuffer, n=bytesavailable(http)) p, nbmax = Base.alloc_request(buf, UInt(n)) - n = Int(GC.@preserve buf unsafe_read(http, p, UInt(min(nbmax, n)))) + GC.@preserve buf (n = unsafe_read(http, p, UInt(min(nbmax, n)))) # TODO: use `Base.notify_filled(buf, n)` here, but only once it is identical to this: if buf.append - buf.size += n + buf.size += Int(n) else - buf.ptr += n + buf.ptr += Int(n) buf.size = max(buf.size, buf.ptr - 1) end return n From 109bf649c25985edc7baf6cdb437552ede092aa9 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 16 Feb 2024 20:25:55 -0500 Subject: [PATCH 4/5] Update Streams.jl --- src/Streams.jl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Streams.jl b/src/Streams.jl index b54cadbc2..a65f88090 100644 --- a/src/Streams.jl +++ b/src/Streams.jl @@ -281,8 +281,9 @@ end function Base.readbytes!(http::Stream, buf::Base.GenericIOBuffer, n=bytesavailable(http)) p, nbmax = Base.alloc_request(buf, UInt(n)) - GC.@preserve buf (n = unsafe_read(http, p, UInt(min(nbmax, n)))) - # TODO: use `Base.notify_filled(buf, n)` here, but only once it is identical to this: + n = min(nbmax, n) + GC.@preserve buf unsafe_read(http, p, UInt(n)) + # TODO: use `Base.notify_filled(buf, Int(n))` here, but only once it is identical to this: if buf.append buf.size += Int(n) else From 6f3353f79c689f865b5d20f978b5042140024904 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 23 Feb 2024 16:43:14 +0000 Subject: [PATCH 5/5] fix some tests relying on implementation details --- src/Streams.jl | 4 ++-- test/client.jl | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Streams.jl b/src/Streams.jl index a65f88090..768965bdb 100644 --- a/src/Streams.jl +++ b/src/Streams.jl @@ -281,7 +281,7 @@ end function Base.readbytes!(http::Stream, buf::Base.GenericIOBuffer, n=bytesavailable(http)) p, nbmax = Base.alloc_request(buf, UInt(n)) - n = min(nbmax, n) + nbmax < n && throw(ArgumentError("Unable to grow response stream IOBuffer $nbmax large enough for response body size: $n")) GC.@preserve buf unsafe_read(http, p, UInt(n)) # TODO: use `Base.notify_filled(buf, Int(n))` here, but only once it is identical to this: if buf.append @@ -321,7 +321,7 @@ function IOExtras.readuntil(http::Stream, f::Function) update_ntoread(http, length(bytes)) return bytes catch ex - ex isa EOFError() || rethrow() + ex isa EOFError || rethrow() # if we error, it means we didn't find what we were looking for # TODO: this seems very sketchy return UInt8[] diff --git a/test/client.jl b/test/client.jl index 0fb2498b6..47dc9baf3 100644 --- a/test/client.jl +++ b/test/client.jl @@ -140,7 +140,7 @@ end # wrapping pre-allocated buffer in IOBuffer will write to buffer directly io = IOBuffer(body; write=true) r = HTTP.get("https://$httpbin/bytes/100"; response_stream=io, socket_type_tls=tls) - @test body === r.body.data + @test Base.mightalias(body, r.body.data) # if provided buffer is too small, we won't grow it for user body = zeros(UInt8, 10) @@ -156,27 +156,30 @@ end # but if you wrap it in a writable IOBuffer, we will grow it io = IOBuffer(body; write=true) r = HTTP.get("https://$httpbin/bytes/100"; response_stream=io, socket_type_tls=tls) - # same Array, though it was resized larger - @test body === r.body.data + # might be a new Array, resized larger + body = take!(io) @test length(body) == 100 # and you can reuse it seekstart(io) r = HTTP.get("https://$httpbin/bytes/100"; response_stream=io, socket_type_tls=tls) - # same Array, though it was resized larger - @test body === r.body.data + # `take!` should have given it a new Array + @test !Base.mightalias(body, r.body.data) + body = take!(io) @test length(body) == 100 # we respect ptr and size body = zeros(UInt8, 100) io = IOBuffer(body; write=true, append=true) # size=100, ptr=1 r = HTTP.get("https://$httpbin/bytes/100"; response_stream=io, socket_type_tls=tls) + body = take!(io) @test length(body) == 200 body = zeros(UInt8, 100) io = IOBuffer(body, write=true, append=false) write(io, body) # size=100, ptr=101 r = HTTP.get("https://$httpbin/bytes/100"; response_stream=io, socket_type_tls=tls) + body = take!(io) @test length(body) == 200 end