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

[ticket/16496] Fix html special chars in custom BBcode help line #5990

Merged
merged 4 commits into from Jun 1, 2020
Merged

[ticket/16496] Fix html special chars in custom BBcode help line #5990

merged 4 commits into from Jun 1, 2020

Conversation

3D-I
Copy link
Contributor

@3D-I 3D-I commented May 25, 2020

PHPBB3-16496

Checklist:

  • Correct branch: master for new features; 3.3.x & 3.2.x for fixes
  • Tests pass
  • Code follows coding guidelines: master, 3.3.x and 3.2.x
  • Commit follows commit message format

Tracker ticket (set the ticket ID to your ticket ID):

https://tracker.phpbb.com/browse/PHPBB3-16496

@3D-I
Copy link
Contributor Author

3D-I commented May 25, 2020

immagine

immagine

@3D-I 3D-I closed this May 25, 2020
@3D-I 3D-I reopened this May 25, 2020
@marc1706
Copy link
Member

Somehow your example does not work as displayed above:
image

@3D-I
Copy link
Contributor Author

3D-I commented May 26, 2020

What commit are you using?

@marc1706
Copy link
Member

Latest in this PR. :)

@3D-I
Copy link
Contributor Author

3D-I commented May 26, 2020

I guess I was right then, wait pls.

@3D-I
Copy link
Contributor Author

3D-I commented May 26, 2020

Data's stored as in

immagine

In the database, thus we need to decode it.

immagine

@kasimi
Copy link
Member

kasimi commented May 27, 2020

  1. BBCode help text entered in the ACP: Same effect as > or < "HTML tags"
  2. The value stored in the database (htmlspecialchars() applied by the type_cast_helper): Same effect as &gt; or &lt; &quot;HTML tags&quot;
  3. htmlspecialchars_decode() applied, as currently proposed in this PR: Same effect as > or < "HTML tags". This is the value of the BBCODE_HELPLINE template variable.
  4. Twig filter e('html_attr') applied in both posting_buttons.html and acp_posting_buttons.html: Same&#x20;effect&#x20;as&#x20;&gt;&#x20;or&#x20;&lt;&#x20;&quot;HTML&#x20;tags&quot;

While this works, I see two problems:

  • There is unescaped data in the template, while usually all data assigned to the template (usernames, post text etc) has their HTML entities encoded. This means that the BBCODE_HELPLINE variable should not be used as is, but needs to be encoded when rendering, not just by the core but also by extension that use it. Again, this is not the default behavior.
  • Why decode the entities in step 3 only to encode them again in step 4? It's perfectly fine to use the value in step 2 in an HTML attribute value without further escaping.

Unless I'm missing something, I'd suggest to remove the code from steps 3 and 4.

@marc1706 can you please test again?

@3D-I
Copy link
Contributor Author

3D-I commented May 27, 2020

Unless I'm missing something, I'd suggest to remove the code from steps 3 and 4.

Unless I am missing something I recall I have already tried it all, I am on phpBB 3.3.x testing here, if that matters. 🤔

Same&#x20;effect&#x20;as&#x20;&gt;&#x20;or&#x20;&lt;&#x20;&quot;HTML&#x20;tags&quot;

Where do you see this?

@kasimi
Copy link
Member

kasimi commented May 27, 2020

It's the final output.

image

@3D-I
Copy link
Contributor Author

3D-I commented May 27, 2020

Never seen here.
I should get some spare time later on tonight to re-test everything though, not an issue.

@kasimi
Copy link
Member

kasimi commented May 27, 2020

Example in the core of a user-controlled title attribute:

@3D-I
Copy link
Contributor Author

3D-I commented May 28, 2020

Wrong okay. Let me push.

@3D-I 3D-I closed this May 28, 2020
@3D-I 3D-I reopened this May 28, 2020
@3D-I 3D-I closed this May 28, 2020
@3D-I 3D-I reopened this May 28, 2020
@3D-I 3D-I closed this May 30, 2020
@3D-I 3D-I reopened this May 30, 2020
@marc1706 marc1706 added this to the 3.2.10 milestone Jun 1, 2020
@marc1706
Copy link
Member

marc1706 commented Jun 1, 2020

Looks good now.

@marc1706 marc1706 merged commit dcae4f3 into phpbb:3.2.x Jun 1, 2020
@3D-I 3D-I deleted the ticket/16496 branch June 1, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants