Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Character lost in HTTP post response body. #1166

Closed
PhyX-Meow opened this issue Apr 11, 2024 · 9 comments
Closed

Character lost in HTTP post response body. #1166

PhyX-Meow opened this issue Apr 11, 2024 · 9 comments

Comments

@PhyX-Meow
Copy link

PhyX-Meow commented Apr 11, 2024

Versions:

  • Julia 1.11.0-beta1
  • HTTP.jl 1.10.5
  • MbedTLS.jl 1.1.9

I have a HTTP server running locally, but querying the server using HTTP.post give me this:

In [9]: HTTP.post("http://127.0.0.1:5700/get_login_info", ["Content-Type" => "application/json"], body = "{}")
HTTP.Messages.Response:
"""
HTTP/1.1 200 OK
Content-Type: application/json
Server: Microsoft-NetCore/2.0
Date: Thu, 11 Apr 2024 11:54:50 GMT
Content-Length: 113

"status":"ok","retcode":0,"data":{"user_id":3785797225,"nickname":"\u609F\u7406\u80DA\u80CE\u7403"},"echo":null}"""

The response body is missing the first character, in my case, the beginning { in json string is lost.
Correct result should look like this curl result:

❯ curl http://127.0.0.1:5700/get_login_info
{"status":"ok","retcode":0,"data":{"user_id":3785797225,"nickname":"\u609F\u7406\u80DA\u80CE\u7403"},"echo":null}

The issue happens today when I updated julia version.

@PhyX-Meow
Copy link
Author

PhyX-Meow commented Apr 11, 2024

This is not the only issue with julia 1.11-beta, running pkg>test HTTP results in a very long list of errors.

@marcelo-g-simas
Copy link

marcelo-g-simas commented Apr 12, 2024

I am seeing a similar issue when processing the body of a POST request, see issue above on the Oxygen.jl repo for details.

@KristofferC
Copy link
Contributor

Does anyone have a simple repro for this?

@marcelo-g-simas
Copy link

Server

using Oxygen
using HTTP
using StructTypes

struct Login
    user_name::String
end

@post "/login" function(req::HTTP.Request)
    @info req.body
    login_data = json(req, Login)
    @info login_data
    return "hello world!"
end

# start the web server
serve()

Client

using HTTP
HTTP.post("http://127.0.0.1:8080/login", body = "{ \"user_name\": \"test\" }")

@marcelo-g-simas
Copy link

marcelo-g-simas commented Apr 18, 2024

You can see what the exception that Client call generates on the Server while running on Julia 1.11 here -> OxygenFramework/Oxygen.jl#186 (comment)

@nhz2
Copy link

nhz2 commented Apr 19, 2024

I think the issue is from

bytes = String(IOExtras.readuntil(io, find_end_of_header))

which calls:

HTTP.jl/src/IOExtras.jl

Lines 115 to 124 in efd74ea

function readuntil(buf::IOBuffer,
find_delimiter::F #= Vector{UInt8} -> Int =#
) where {F <: Function}
l = find_delimiter(view(buf.data, buf.ptr:buf.size))
if l == 0
return nobytes
end
bytes = view(buf.data, buf.ptr:buf.ptr + l - 1)
buf.ptr += l
return bytes

Since
JuliaLang/julia#53896

calling String on a view of a Memory and then trying to use the Memory is unsafe.

Here is a example:

julia> a = Memory{UInt8}(undef, 10);

julia> fill!(a, 0x01);

julia> String(view(a, 1:2))
"\x01\x01"

julia> a
0-element Memory{UInt8}

@KristofferC
Copy link
Contributor

Yeah, I think all the uses of the IOBuffer internals has to be removed.

#1145 and #1147 has started that work.

@vtjnash
Copy link
Member

vtjnash commented Apr 19, 2024

In theory, a normal readbyte call there could allocate a StringVector more directly here, avoiding the copy later to make a String from this, and avoid relying on implementation details too

quinnj added a commit that referenced this issue Apr 20, 2024
@quinnj
Copy link
Member

quinnj commented Apr 20, 2024

I agree we need to refactor a bunch of the internals to not abuse IOBuffer; either move everything to be Memory-based (my preference since we'll have full control and want to be efficient as possible w/ # of allocations), or just use IOBuffer's properly. In any case, I think #1170 is at least a stop-gap for the immediate issue posted here.

@quinnj quinnj closed this as completed in 7074172 Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants