Skip to content

257 Request URI query string validation#288

Merged
simonoakesepimorphics merged 2 commits into
mainfrom
257-query-validation
May 21, 2026
Merged

257 Request URI query string validation#288
simonoakesepimorphics merged 2 commits into
mainfrom
257-query-validation

Conversation

@simonoakesepimorphics
Copy link
Copy Markdown
Contributor

Fixes #257
Was unable to replicate the issue on the current dev version of ELDA. Added error handling to mitigate this just in case.

@simonoakesepimorphics simonoakesepimorphics requested a review from kal May 14, 2026 16:26
@simonoakesepimorphics simonoakesepimorphics changed the base branch from main to 235-exception-handling May 14, 2026 16:27
Copy link
Copy Markdown
Contributor

@kal kal left a comment

Choose a reason for hiding this comment

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

One issue to check

result.add(pair[0], pair[1]);
if (pair.length > 1) {
result.add(pair[0], pair[1]);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this add pair[0] if the query parameter is just a name with no value. I believe that is allowed to indicate a boolean true.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would be good to also have this branch covered by a test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This code is itself a test utility - I suggest it's outside the scope of what we would normally unit test.

Base automatically changed from 235-exception-handling to main May 18, 2026 14:57
@simonoakesepimorphics simonoakesepimorphics requested a review from kal May 19, 2026 15:09
result.add(pair[0], pair[1]);
if (pair.length > 1) {
result.add(pair[0], pair[1]);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would be good to also have this branch covered by a test

Copy link
Copy Markdown
Contributor

@kal kal left a comment

Choose a reason for hiding this comment

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

Ah ok, I hadn't clocked that the code was in the test branch.

@simonoakesepimorphics simonoakesepimorphics merged commit bbaa75d into main May 21, 2026
2 checks passed
@simonoakesepimorphics simonoakesepimorphics deleted the 257-query-validation branch May 21, 2026 08:51
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.

Incomplete query validation

2 participants