Skip to content

Conversation

@scw00
Copy link
Member

@scw00 scw00 commented Jun 19, 2017

Missing Length if format was set in fabricate_with_old_api

The resulting_buffer_length will be zero if format was set. Then ats free the return's buffer and close connection directly.

…ld_api

The resulting_buffer_length will be zero if format was set.
@scw00
Copy link
Member Author

scw00 commented Jun 19, 2017

test script

function do_remap()
    ts.http.set_resp(403, "403 Forbidden")
    return 0
end

response

> POST HTTP://www.scw.com/bjh HTTP/1.0
> User-Agent: curl/7.35.0
> Host: www.scw.com
> Accept: */*
> Proxy-Connection: Keep-Alive
> Content-Length: 65721
> Content-Type: multipart/form-data; boundary=------------------------eb5732dcfe4fef73
> 
} [data not shown]
* HTTP 1.0, assume close after body
< HTTP/1.0 403 Forbidden
< Date: Mon, 19 Jun 2017 01:57:26 GMT
< Via: http/1.1 ubuntu-linux (ApacheTrafficServer/8.0.0 [c s f ])
< Server: ATS/8.0.0
< Cache-Control: no-store
< Content-Type: text/html
< Content-Language: en
< Content-Length: 13
< 
{ [data not shown]
* Closing connection 0

@bryancall
Copy link
Contributor

[approve ci]

@zwoop
Copy link
Contributor

zwoop commented Jun 19, 2017

@scw00 do you happen to know what version(s) of ATS this affects? We need this in 7.1.0 ?

@zwoop zwoop added this to the 8.0.0 milestone Jun 19, 2017
@scw00
Copy link
Member Author

scw00 commented Jun 20, 2017

No, it is only in 7.2 and 8.0 since #1299 merged.

@SolidWallOfCode SolidWallOfCode self-requested a review June 26, 2017 13:42
Copy link
Member

@SolidWallOfCode SolidWallOfCode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original use of plain_flag is very odd - it's initialized to false and then never changed. Presumably it was intended for this use?

@scw00 scw00 merged commit 3387c0f into apache:master Jul 1, 2017
@scw00 scw00 deleted the body_factory branch July 28, 2017 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants