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

nl2br ignores \r in 3.3.0 #3815

Closed
Majkl578 opened this issue Sep 22, 2014 · 5 comments
Closed

nl2br ignores \r in 3.3.0 #3815

Majkl578 opened this issue Sep 22, 2014 · 5 comments

Comments

@Majkl578
Copy link
Contributor

echo nl2br("Hello<br />\nmy<br />\r\nfriend<br />\n\r", TRUE);

On PHP, the output is correct:

Hello<br /><br />
my<br /><br />
friend<br /><br />

On HHVM, output is broken and even tags are broken:

Hello<br /><br />
<br />/>
friend<br /><br />

And even worse if 2nd parameter is FALSE (no XHTML) - still generates XHTML, but broken even more:

Hello<br /><br>
<br>r />
friend<br /><br>

On 3.2.0 and earlier, output was wrong as well, but never broken:

Hello<br /><br />
my<br />
<br />
friend<br /><br />
@SiebelsTim
Copy link
Contributor

@ptarjan
Copy link
Contributor

ptarjan commented Sep 22, 2014

Yeah, we screw up on \r. Compare http://3v4l.org/olqqs http://3v4l.org/6pgnv

@Majkl578
Copy link
Contributor Author

Yeah, that would not be a big deal. But the major problem is that it leads to a data loss; look at the output in 3.3.0, "my" is completely missing and one tag is partially lost as well. I guess that, under some circumstances, it might be even considered as a security problem.

@MaideCa
Copy link
Contributor

MaideCa commented Sep 23, 2014

I believe your data loss is actually just the carriage return moving the printing cursor to the start of the line and then you write over your other characters.

@Majkl578 Majkl578 changed the title nl2br output seriously broken in 3.3.0 nl2br ignores \r in 3.3.0 Sep 23, 2014
@Majkl578
Copy link
Contributor Author

@MaideCa: Right, that would be the case. :) It got split from \n and became interpreted in output in a confusing way...

rrh pushed a commit to newrelic-forks/hhvm that referenced this issue Sep 24, 2014
Summary: nl2br() now matches PHP to support prepending of html new lines on carriage returns in addition to newlines.  We also handle special cases when there is a carriage return beside a newline; each pair only get one html new line instead of two.

Fixes facebook#3815

Closes facebook#3821

Reviewed By: @ptarjan

Differential Revision: D1571327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants