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.

Sign up for GitHub

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

Jump to bottom

[ticket/16524] Fix using out-of-bounds UTF8 characters in profile fields #6006

Merged
merged 5 commits into from
Jul 22, 2020
Merged

[ticket/16524] Fix using out-of-bounds UTF8 characters in profile fields #6006

merged 5 commits into from
Jul 22, 2020

Conversation

rxu
Copy link
Contributor

@rxu rxu commented Jun 10, 2020 •
edited
Loading

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

PHPBB3-16524.

3D-I
3D-I suggested changes Jun 10, 2020
View reviewed changes
Copy link
Contributor

@3D-I 3D-I left a comment

Choose a reason for hiding this comment

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

Great, I like it. 👍 Not tested.

Just the language at a first glance should be amended IMO.

@rxu
Copy link
Contributor Author

rxu commented Jun 10, 2020

Probably should be rebased on 3.3.x as well.

@3D-I
Copy link
Contributor

3D-I commented Jun 10, 2020

Probably should be rebased on 3.3.x as well.

Oh well, that is also good news, 😄 I am assuming somehow (not tested) that this PR will break my poor extension lol. But I just do not care do not worry.

@rxu rxu changed the base branch from 3.2.x to 3.3.x June 10, 2020 15:48
@3D-I
Copy link
Contributor

3D-I commented Jun 14, 2020

I propose we use a new permission for this, or none at all.

It is better to distinguish the two things, also to avoid criticism as we have already had in the past.

@rxu
Copy link
Contributor Author

rxu commented Jun 15, 2020

Currently used permission has a generic id u_emoji so creating another emoji-specific one would be redundant. This way it would end up with own emoji permission for every single text field.

@3D-I
Copy link
Contributor

3D-I commented Jun 15, 2020 •
edited
Loading

This way it would end up with own emoji permission for every single text field.

Exactly, currently only the topic title has permissions for Emoji.
Not any other text field all over the phpBB. We have already covered quite the whole lot.
That's why I think it's best to remove the permission check altogether for this as well

@marc1706 marc1706 changed the base branch from 3.3.x to prep-release-3.2.10 July 21, 2020 09:13
@marc1706
Copy link
Member

Rebased onto prep-release-3.2.10 to also have it in that ;)

@3D-I
Copy link
Contributor

3D-I commented Jul 21, 2020

No milestone?

@marc1706 marc1706 added this to the 3.2.10 milestone Jul 21, 2020
@marc1706 marc1706 merged commit df49e0e into phpbb:prep-release-3.2.10 Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marc1706 marc1706 marc1706 left review comments

@3D-I 3D-I 3D-I approved these changes

Assignees
No one assigned
Labels
3.2 (Rhea)
Projects
None yet
Milestone
3.2.10
Development

Successfully merging this pull request may close these issues.
3 participants
@rxu @3D-I @marc1706