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

Encode strings as UTF-8 when it has wide characters #429

Merged
merged 1 commit into from
Sep 23, 2013

Conversation

miyagawa
Copy link
Member

@miyagawa miyagawa commented Sep 8, 2013

This has been controversial for so long, and my canned response is that an app returning data that breaks PSGI specification is ok to break, and should these kind of errors be handled by Lint in the development.

Meanwhile, a) realistically it's not cool for app mistakes to be able to crash servers and b) if some of these PSGI violation only happens sometimes in the runtime, catching them with Lint might be difficult as well.

There should always be a line drawn, since there are many other ways to break HTTP::Server::PSGI as well by returning (for example) a hash for headers instead of arrays, and handling all of these mistakes in the server isn't realistic (and could make the server slow).

This should probably be documented as part of the spec or guideline, but here's a quick fix on the reference server to catch such errors.

> perl -S plackup  -E dev -e ' sub{[200,["Content-Type","text/plain"],["\x{3092}"]]}'
Wide character in syswrite at /Users/miyagawa/.plenv/versions/5.18.1/lib/perl5/5.18.1/darwin-2level/IO/Handle.pm line 481.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) when pulling 86602ce on server-encode-utf8 into 365d440 on master.

miyagawa added a commit that referenced this pull request Sep 23, 2013
Encode strings as UTF-8 when it has wide characters
@miyagawa miyagawa merged commit 6053f8c into master Sep 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants