HTTP.jl icon indicating copy to clipboard operation
HTTP.jl copied to clipboard

`readbytes!` on `HTTP` stream and `IOBuffer` should return the number of read bytes

Open rfourquet opened this issue 3 years ago • 1 comments

This is what readbytes! docstring from Base says, but readbytes! on a HTTP streams returns the total number of bytes. I realize that this method is probably internal as it takes an IOBuffer, so feel free to close this issue, but it wouldn't be a difficult change to update it, it could be:

function Base.readbytes!(http::Stream, buf::IOBuffer, n=bytesavailable(http))
    Base.ensureroom(buf, n)
    unsafe_read(http, pointer(buf.data, buf.size + 1), n)
    buf.size += n
    n # added line
end

Also, I don't know whether this function is used safely internally in HTTP, but for a given IOBuffer which is not empty, this will segfault easily, as Base.ensureroom takes only buf.ptr into account, but only buf.size is updated in the above readbytes! method, e.g.

using HTTP
buf = IOBuffer()
HTTP.open("GET", "http://httpbin.org/stream/10") do io
    while !eof(io)
        k = bytesavailable(io)
        n = readbytes!(io, buf, k)
        @show k, n, length(buf.data), buf.size, buf.ptr
    end
end

gives

(k, n, length(buf.data), buf.size, buf.ptr) = (516, 774, 516, 774, 1)
(k, n, length(buf.data), buf.size, buf.ptr) = (516, 1290, 516, 1290, 1)
(k, n, length(buf.data), buf.size, buf.ptr) = (258, 1548, 516, 1548, 1)
(k, n, length(buf.data), buf.size, buf.ptr) = (516, 2064, 516, 2064, 1)
(k, n, length(buf.data), buf.size, buf.ptr) = (258, 2322, 516, 2322, 1)
(k, n, length(buf.data), buf.size, buf.ptr) = (258, 2580, 516, 2580, 1)
(k, n, length(buf.data), buf.size, buf.ptr) = (0, 2580, 516, 2580, 1)

signal (11): Segmentation fault
in expression starting at none:0
[...]

rfourquet avatar Aug 26 '22 15:08 rfourquet

You're correct that this is more of an internally used definition, the only usage being:

function Base.read(http::Stream)
    buf = PipeBuffer()
    if ntoread(http) == unknown_length
        while !eof(http)
            readbytes!(http, buf)
        end
    else
        while !eof(http)
            readbytes!(http, buf, ntoread(http))
        end
    end
    return take!(buf)
end

I'd take a PR cleaning this up though.

quinnj avatar Aug 30 '22 04:08 quinnj