Skip to content

TS-5103: replace ua_raw_buffer_reader with ua_buffer_reader#1271

Merged
shinrich merged 1 commit intoapache:masterfrom
oknet:TS-5103
Jan 6, 2017
Merged

TS-5103: replace ua_raw_buffer_reader with ua_buffer_reader#1271
shinrich merged 1 commit intoapache:masterfrom
oknet:TS-5103

Conversation

@oknet
Copy link
Copy Markdown
Member

@oknet oknet commented Dec 22, 2016

No description provided.

@atsci
Copy link
Copy Markdown

atsci commented Dec 22, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1275/ for details.

@atsci
Copy link
Copy Markdown

atsci commented Dec 22, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/1170/ for details.

@bryancall bryancall added the HTTP label Dec 22, 2016
@bryancall bryancall modified the milestones: 7.1.1, 7.1.0 Dec 22, 2016
@bryancall bryancall self-requested a review December 22, 2016 16:18
@bryancall bryancall self-assigned this Dec 22, 2016
@shinrich
Copy link
Copy Markdown
Member

shinrich commented Jan 4, 2017

Yes, this looks like the correct change to me. We should be checking that there is more data on the ua_buffer_reader. In this case, there should always be data on the ua_raw_buffer_reader, so we will be incorrectly blind tunneling GET requests without keep alive enabled. (I think).

We only want to flip to tunnel mode if there is data after the already parsed header which would be reflected by the ua_buffer_reader.

@shinrich shinrich self-requested a review January 4, 2017 14:17
Copy link
Copy Markdown
Member

@shinrich shinrich left a comment

Choose a reason for hiding this comment

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

See my previous comment (figuring out this review flow..) Looks like a good fix to me.

@shinrich shinrich merged commit b4f551b into apache:master Jan 6, 2017
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