Skip to content

fix(caniuse): handle wf- prefixed web-features IDs#494

Merged
marcoscaceres merged 16 commits into
mainfrom
fix/caniuse-web-features
Jun 3, 2026
Merged

fix(caniuse): handle wf- prefixed web-features IDs#494
marcoscaceres merged 16 commits into
mainfrom
fix/caniuse-web-features

Conversation

@marcoscaceres

Copy link
Copy Markdown
Collaborator

Summary

  • Strip wf- prefix and retry when caniuse feature lookup fails
  • Return structured JSON 404 with helpful hint for web-features IDs
  • Only silence ENOENT errors, log everything else

Closes #467

Comment thread routes/caniuse/lib/index.ts Fixed
@marcoscaceres
marcoscaceres force-pushed the fix/caniuse-web-features branch from 5ed688e to b350086 Compare April 25, 2026 12:22
Comment thread routes/caniuse/lib/index.ts Fixed

Copilot AI left a comment

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.

Pull request overview

Updates the caniuse feature lookup to better support caniuse’s newer wf- (web-features) identifiers and improves not-found responses.

Changes:

  • Add fallback lookup logic that strips the wf- prefix when an initial caniuse feature lookup fails.
  • Return a JSON 404 response from the /caniuse/:feature route with an actionable hint when wf- is used.
  • Suppress logging for ENOENT (missing feature file) while continuing to log other read/parse errors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
routes/caniuse/lib/index.ts Adds wf- fallback lookup and refines error logging for missing feature files.
routes/caniuse/feature.ts Returns a structured JSON 404 with a wf--specific hint; removes an unused import.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread routes/caniuse/lib/index.ts Outdated

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread routes/caniuse/lib/index.ts
Comment thread routes/caniuse/lib/index.ts Outdated
Comment thread routes/caniuse/lib/index.ts Outdated

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread routes/caniuse/lib/index.ts Outdated

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@marcoscaceres
marcoscaceres force-pushed the fix/caniuse-web-features branch 2 times, most recently from 4e21c0a to 5aa29ab Compare April 26, 2026 01:07
@marcoscaceres
marcoscaceres requested a review from Copilot April 26, 2026 01:20

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread routes/caniuse/lib/index.ts Outdated
Comment thread routes/caniuse/feature.ts Outdated
Comment thread routes/caniuse/feature.ts
When a caniuse feature lookup fails with a wf- prefix, strip the prefix and
retry. Returns a clear 404 if no match is found.

Closes #467
Comment thread routes/caniuse/lib/index.ts Fixed
Comment thread routes/caniuse/lib/index.ts Fixed

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread routes/caniuse/lib/index.ts Outdated
@marcoscaceres

Copy link
Copy Markdown
Collaborator Author

@copilot Can you review this and fix any issues that come up?

@marcoscaceres

Copy link
Copy Markdown
Collaborator Author

@copilot can you confirm all your feedback has been addressed and resolved everything that has? Can you fix anything that wasn’t addressed and resolve those comments.

Copilot AI commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

@copilot can you confirm all your feedback has been addressed and resolved everything that has? Can you fix anything that wasn’t addressed and resolve those comments.

All feedback has been addressed. Here's a complete summary:

Security fixes

  • Path traversal: getData() validates feature names with /^[a-z0-9][a-z0-9._-]*$/i before using them in file paths (commit 73a407e)
  • XSS: HTML 404 messages in index.ts are escaped with escape-html (commit db579de)
  • CodeQL type confusion: runtime typeof feature !== "string" guard added in getData() (commit 2c03d80)

Logic improvements

  • wf- fallback only runs when feature.length > 3 to avoid empty-string file lookups
  • Successful wf- fallback is cached under the original key to avoid repeated disk I/O
  • readFeatureFile only silences ENOENT; JSON parse/permission errors are rethrown as 500s
  • browsers=all sentinel is passed through before splitting in the legacy route
  • Legacy route respects format=html on error paths (HTML 404) vs JSON format (JSON 404)

Consistent JSON error shapes

  • feature.ts 404 and catch block both return { error: "..." } JSON (catch now returns 500, not 404)
  • index.ts legacy route 404 returns JSON when format=json, HTML when format=html

Test coverage (all 55 specs pass, CI Tests ✅ CodeQL ✅)

  • tests/routes/caniuse/lib/index.test.js: 9 specs (getData wf- fallback, caching, path traversal, edge cases)
  • tests/routes/caniuse/feature.test.js: 6 specs (404 shape, wf- hint, 500 on malformed JSON, successful responses)

@marcoscaceres
marcoscaceres marked this pull request as ready for review April 27, 2026 19:37
@marcoscaceres
marcoscaceres requested a review from sidvishnoi May 4, 2026 11:17
Comment thread routes/caniuse/index.ts
@marcoscaceres

Copy link
Copy Markdown
Collaborator Author

@copilot Can you rebase this on main and resolve the merge conflicts?

- Merges all recent main branch commits (TypeScript 6, baseline API,
  xref improvements, misc/cache tests, etc.)
- Reverts deprecated GET /caniuse?feature= route to its original form
  per @sidvishnoi's review (route is deprecated, keep as-is)
- Removes escape-html direct dependency (only needed for deprecated
  route changes that were reverted; still available as transitive dep
  of express)
- Resolves merge conflicts in feature.ts and pnpm-lock.yaml

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/1c7aab3b-9248-4f65-950f-099f23b327e7

Copilot AI commented May 5, 2026

Copy link
Copy Markdown
Contributor

@copilot Can you rebase this on main and resolve the merge conflicts?

Done in commit 742c251. Merged origin/main into the branch (TypeScript 6, baseline API, xref improvements, etc.), resolved conflicts in routes/caniuse/feature.ts and pnpm-lock.yaml, and pushed. All 141 specs pass, CodeQL 0 alerts.

@marcoscaceres

Copy link
Copy Markdown
Collaborator Author

@copilot can you fix the merge conflicts?

Resolves conflict in routes/caniuse/feature.ts: keep JSON 500 error
response (our improvement) over the old text/plain response from main.

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>

Copilot AI commented May 5, 2026

Copy link
Copy Markdown
Contributor

@copilot can you fix the merge conflicts?

Fixed in commit 48c7ce6. The previous merge (742c251) wasn't a proper two-parent merge commit, so GitHub still saw conflicts. This commit is a true merge of origin/main with the single conflict in feature.ts (catch block) resolved by keeping our JSON 500 response. All 141 specs pass.

@marcoscaceres
marcoscaceres requested a review from sidvishnoi May 19, 2026 02:11
@marcoscaceres
marcoscaceres merged commit 354993c into main Jun 3, 2026
3 checks passed
@marcoscaceres
marcoscaceres deleted the fix/caniuse-web-features branch June 3, 2026 14:59
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.

can I use route: Can't seem to match digital credentials

5 participants