- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 4.6k
Fix non-ascii characters present in the header #30614
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
Conversation
| @Marek-Wojtowicz can you fix the DCO please? | 
The http headers according to rfc 2616 is iso-8859-1. This patch fixes the behavior when non-ascii characters are present in the header. Signed-off-by: Marek Wójtowicz <Marek.Wojtowicz@agh.edu.pl>
| I think I fixed the DCO 😄 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks sensible but I'm not familiar enough with how the session token works inside nextcloud to say if this might or not break anything
| An SQL query is fired, inserting a token into the oc_authtoken table. Specifically, the userAgent column has a wrong character encoding (MySQL returns: Incorrect string value). | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 | 
| Backports? | 
| return false; | ||
| } | ||
| $name = isset($request->server['HTTP_USER_AGENT']) ? $request->server['HTTP_USER_AGENT'] : 'unknown browser'; | ||
| $name = isset($request->server['HTTP_USER_AGENT']) ? utf8_encode($request->server['HTTP_USER_AGENT']) : 'unknown browser'; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utf8_encode is a wrongly named php function, I’d feel better if either we have our own alias named iso-8859-1_to_utf8(), or if we use mb_convert_encoding instead (only possible if we depend upon mbstring).
There are frequent proposal to remove utf8_encode from PHP, all were rejected so far, but it shows this function is confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there will be time to fix the problem as utf8_encode becomes obsolete. Until then, there may be newer mechanisms that can replace utf8_encode.
| /backport to stable23 | 
| /backport to stable22 | 
| /backport to stable21 | 
The http headers according to rfc 2616 is iso-8859-1. This patch fixes the behavior when non-ascii characters are present in the header.