-
-
Notifications
You must be signed in to change notification settings - Fork 730
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
iso-8859-1/windows-1252 support? #194
Comments
@dougwilson Any update on this ? I'm interested in this feature but as @papandreou 's branch is way behind the official master. Seems like it's going to be hard to pull... Is there any proper other way to handle ISO-8859-1 encoded data ? |
Hi @Xtrem65 I haven't even commented on here, let alone done any work on it :) I don't have any updates. |
@Xtrem65, I guess the only update is that you're expressing interest, which is also a good thing :) If @dougwilson and @ljharb think the work looks good and they want to adopt this feature, I'd be willing to bring the branches up to date. |
@Xtrem65, have you tried switching to my |
I’d be happy to review a PR. |
@ljharb, that sounds great. Before I try to rebase it on qs master, does this approach look good? papandreou/qs@9250c4c...interpretNumericEntities |
I'd need to review it when rebased :-) it looks like a good direction tho. |
Rebased branch: https://github.com/papandreou/body-parser/tree/feature/iso-8859-1/take2 Requires ljharb/qs#268 to be |
Wow guys ! so much reactivity :D I'd be happy to help if needed |
@Xtrem65, you can help by trying it out :)
... Then see if you can solve your problem. Depending on the requests that you need to support, you might have to specify the |
Closing as niche interest. Looks like some code was written to get folks who want this started. If someone wants to revive this and open a PR, we can get the defibrillator out on this issue. |
indeed, the qs code for this is in v6.6.0+, so only body-parser would need changes. |
@jonchurch, it's implemented by this PR, which has been sitting there for almost 5 years, waiting for a new major release that ditches support for node.js 0.10 and below :) |
apologies @papandreou I missed that PR link while triaging on mobile. I thought a PR never came through, will open this back up as there's a PR attached. Getting up to speed on this request, as I have never had to deal with this problem. But Im all for better supporting common bodies on the internet via config options. Reopening |
Hi,
A while ago I developed a web app to replace an old cgi-bin script from the nineties, which means that it had to assume that POST requests with a
Content-Type
ofapplication/x-www-urlencoded
are iniso-8859-1
(and also support "smart quotes" etc., aka. windows-1252). At the same time it had to be possible to switch toutf-8
mode.I accomplished that by adding support for a
defaultCharset
option, which can either beutf-8
oriso-8859-1
, and for explicitly switching toutf-8
mode by addingutf8=%E2%9C%93
(utf8=✓
as percent-encoded utf-8 octets) to the query string, a trick that I've seen others use as well.Here are the diffs between my forks of body-parser and the qs module:
master...papandreou:iso-8859-1
ljharb/qs@master...papandreou:interpretNumericEntities
Would anyone be interested in this work? If so, I can clean it up and open PRs.
The text was updated successfully, but these errors were encountered: