Skip to content

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Oct 26, 2021

This is another attempt to fix what was initially addressed in #8096 but
got backed out via #8305. That more extensive patch was considered too
invasive and potentially risky. This more targeted patch will fix
clients that only send the \n endings but it will force the \r\n line
ending on output.

This was mostly in place except for header lines that get
m_n_v_raw_printable set, which seems to be most header lines. The
addition checks to see if the header line ends in \r\n. If it does not
the m_n_v_raw_printable flag gets cleared and the logic that explicitly
adds the line endings while be invoked on output.

This is another attempt to fix what was initially addressed in apache#8096 but
got backed out via apache#8305. That more extensive patch was considered too
invasive and potentially risky.  This more targeted patch will fix
clients that only send the \n endings but it will force the \r\n line
ending on output.

This was mostly in place except for header lines that get
m_n_v_raw_printable set, which seems to be most header lines. The
addition checks to see if the header line ends in \r\n. If it does not
the m_n_v_raw_printable flag gets cleared and the logic that explicitly
adds the line endings while be invoked on output.
@bneradt bneradt added the HTTP label Oct 26, 2021
@bneradt bneradt added this to the 10.0.0 milestone Oct 26, 2021
@bneradt bneradt self-assigned this Oct 26, 2021
@bneradt bneradt merged commit 64f2567 into apache:master Oct 27, 2021
@bneradt bneradt deleted the fix_output_line_endings branch October 27, 2021 18:35
bryancall pushed a commit that referenced this pull request Oct 27, 2021
This is another attempt to fix what was initially addressed in #8096 but
got backed out via #8305. That more extensive patch was considered too
invasive and potentially risky.  This more targeted patch will fix
clients that only send the \n endings but it will force the \r\n line
ending on output.

This was mostly in place except for header lines that get
m_n_v_raw_printable set, which seems to be most header lines. The
addition checks to see if the header line ends in \r\n. If it does not
the m_n_v_raw_printable flag gets cleared and the logic that explicitly
adds the line endings while be invoked on output.

(cherry picked from commit 64f2567)
@bryancall bryancall modified the milestones: 10.0.0, 9.1.1 Oct 27, 2021
ezelkow1 pushed a commit to ezelkow1/trafficserver that referenced this pull request Nov 3, 2021
This is another attempt to fix what was initially addressed in apache#8096 but
got backed out via apache#8305. That more extensive patch was considered too
invasive and potentially risky.  This more targeted patch will fix
clients that only send the \n endings but it will force the \r\n line
ending on output.

This was mostly in place except for header lines that get
m_n_v_raw_printable set, which seems to be most header lines. The
addition checks to see if the header line ends in \r\n. If it does not
the m_n_v_raw_printable flag gets cleared and the logic that explicitly
adds the line endings while be invoked on output.
zwoop pushed a commit that referenced this pull request Nov 8, 2021
This is another attempt to fix what was initially addressed in #8096 but
got backed out via #8305. That more extensive patch was considered too
invasive and potentially risky.  This more targeted patch will fix
clients that only send the \n endings but it will force the \r\n line
ending on output.

This was mostly in place except for header lines that get
m_n_v_raw_printable set, which seems to be most header lines. The
addition checks to see if the header line ends in \r\n. If it does not
the m_n_v_raw_printable flag gets cleared and the logic that explicitly
adds the line endings while be invoked on output.

(cherry picked from commit 64f2567)
@zwoop
Copy link
Contributor

zwoop commented Nov 8, 2021

Cherry-picked to v9.2.x

@zwoop zwoop added the 9.2.0 label Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants