[p5.js 2.0+ Bug Report]: getURLParams() mis-parses valueless parameters and never percent-decodes values
メンテナーはふだん 3 日以内に返信
評価
- 難易度
- 2/5
- 見積もり時間
- 1〜3時間
- 初心者へのやさしさ
- 88/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- javascript
- 領域
- frontend
調査の方向性
src/core/environment.js:1058 の getURLParams から始めて、issue の 4 つのスタンドアロンケース(インライン regex スニペット)を実行し、何も触る前に誤パースを確認します。提案されている置き換えは Object.fromEntries(new URLSearchParams(location.search)) で、値なしキー、パーセントデコード、+ をスペースとして扱います。それに伴い lastIndex のワークアラウンドは削除します。environment のユニットテストの下に ?debug&x=1、?a=1&b、?name=Hello%20World、?q=a+b の回帰テストを追加し、getURLParams のリファレンスドキュメントで double-decode の注記を確認します。
索引モデルが issue の本文から書いたものです。
説明
Most appropriate sub-area of p5.js?
- Core/Environment/Rendering
p5.js version
2.3.2 (verified on main @ 94fb07d)
Web browser and version
All — the parser is a hand-rolled regex, not a browser API.
Operating system
All
Steps to reproduce this
Steps:
- Open any sketch with
?debug&x=1appended to the URL. - Log
getURLParams(). - It returns
{ debug: "x" }— a pair that does not exist in the query string — andx=1is gone entirely. - Try
?name=Hello%20World: the value comes back still percent-encoded.
Snippet:
function setup() {
print(getURLParams());
}
The parser can be run standalone to see all the cases:
const re = /[?&]([^&=]+)(?:[&=])([^&=]+)/gim;
function current(search) {
let m; const v = {};
while ((m = re.exec(search)) != null) {
if (m.index === re.lastIndex) re.lastIndex++;
v[m[1]] = m[2];
}
return v;
}
current('?debug&x=1'); // { debug: "x" } expected { debug: "", x: "1" }
current('?a=1&b'); // { a: "1" } expected { a: "1", b: "" }
current('?name=Hello%20World'); // { name: "Hello%20World" } expected { name: "Hello World" }
current('?q=a+b'); // { q: "a+b" } expected { q: "a b" }
Cause
// src/core/environment.js:1058
fn.getURLParams = function () {
const re = /[?&]([^&=]+)(?:[&=])([^&=]+)/gim;
let m;
const v = {};
while ((m = re.exec(location.search)) != null) {
if (m.index === re.lastIndex) {
re.lastIndex++;
}
v[m[1]] = m[2];
}
return v;
};
Three problems:
- The separator group accepts
&as well as=.(?:[&=])means a parameter with no value matches&debug&xas keydebug, separator&, valuex— it absorbs the next parameter's name as its own value, and consumes the realx=1pair along with it. A valueless parameter does not just get skipped; it corrupts its neighbour. - Nothing is decoded. No
decodeURIComponent, and+is not translated to a space, so any value containing a space,&,/or a non-ASCII character arrives in its encoded form. - A repeated key keeps only the last occurrence, and the returned object is a
{}literal, so it inheritstoString,constructor,hasOwnPropertyand friends. Code that testsif (params.constructor)gets a surprising truthy value.
Expected behaviour
Query strings should be parsed per the URL standard: values decoded, + treated as a space, and a parameter with no value reported as an empty string rather than absorbing the parameter after it.
Suggested fix
The platform already does all of this correctly, and URLSearchParams has been available in every browser p5 2.x supports for years:
fn.getURLParams = function () {
return Object.fromEntries(new URLSearchParams(location.search));
};
That also removes the lastIndex workaround (needed only because of the zero-length-match hazard in the hand-rolled loop) and the prototype-inherited keys.
Two notes on compatibility, since this is a behaviour change and not purely a fix:
- Valueless parameters start appearing as
''instead of being mis-parsed. Any sketch relying on the current output for?flagwas getting wrong data, but it is still a change worth a line in the release notes. - Values arrive decoded. A sketch that was calling
decodeURIComponent()on the result itself would now double-decode. Worth mentioning in the reference docs forgetURLParams().
If you'd prefer to preserve repeated keys as arrays while you're in there, that is easy to add on top — but it is a separate decision, so I've left it out of the suggestion.
I'd be glad to open a PR with the replacement and tests covering all four cases above.
- 主要言語
- JavaScript
- スター
- 24.1k
- フォーク
- 3.9k
- 平均マージ
- 3日 43分
- マージ済み PR(30日)
- 33
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートあり
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
processing/p5.js のほかの issue
-
p5.js 2.0+
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
processing/p5.js#9295 · コメント 1 件 ·
メンテナーはふだん 3 日以内に返信
-
Add unit tests for noiseDetail()対応中かも @harshiltewari2004 が 2 日前に担当しました。 オープンArea:Math Enhancement
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
processing/p5.js#9253 · コメント 2 件 · 担当者 1 名 ·
メンテナーはふだん 3 日以内に返信
-
[p5.js 2.0+ Bug Report]: storeItem() validation branches are missing `return`, so bad input throws a TypeError or is stored anyway対応中かも @iUjjwalRaj が 10 日前に担当しました。 オープン
難易度 1/5 1時間未満 初心者へのやさしさ 85/100
processing/p5.js#9242 ·
メンテナーはふだん 3 日以内に返信
-
[p5.js 2.0+ Bug Report]: Typo in Spanish reference documentation for ellipseMode()再び着手できるかも @cgutierrezval が 13 日前に担当しましたが、オープン中のプルリクエストはありません。 オープンInternationalization p5.js 2.0+
難易度 1/5 1時間未満 初心者へのやさしさ 95/100
processing/p5.js#9231 · コメント 3 件 ·
メンテナーはふだん 3 日以内に返信
-
[p5.js 2.0+ Bug Report]: ReferenceError: p5 is not defined when calling loadPixels/get/copy/mask on p5.MediaElement in ESM対応中かも @Pcmhacker-piro が 18 日前に担当しました。 オープンArea:Core Area:DOM
難易度 2/5 1〜3時間 初心者へのやさしさ 86/100
processing/p5.js#9189 · コメント 1 件 ·
メンテナーはふだん 3 日以内に返信
processing/p5.js の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
メンテナーはふだん 1 日以内に返信
-
needs triage
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
pbakaus/impeccable#1055 ·
メンテナーはふだん 1 日以内に返信
-
Add: EUROTV NEWSオープンchannels:add check:passed
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
メンテナーはふだん 1 日以内に返信