feat: scanner protocol-less urls#1498
Conversation
WalkthroughA new ChangesBarcode scan URL parsing and validation
Security Recommendations🔒 Verification of URL origin validation:
🔒 Case-insensitive route matching:
🔒 Null handling in integration points:
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rufusutt
changed the title
rufusutt
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/composables/use-barcode-detector.ts`:
- Around line 89-96: The regex character class used to sanitize and extract IDs
(the patterns applied to sanitized and in the assetMatch/itemMatch/locationMatch
tests) incorrectly places the hyphen between 9 and _, creating an unintended
range; update those regexes (the sanitization regex and the three extraction
regexes in useBarcodeDetector/composable where sanitized.match is used) to
either escape the hyphen or move it to the end of the class (e.g., change
[a-zA-Z0-9-_] to [A-Za-z0-9_-] or [A-Za-z0-9_\-]) so IDs only match letters,
digits, underscore and hyphen.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1116ca53-7d0a-4c59-bb58-536ea224090b
📒 Files selected for processing (4)
frontend/components/App/ScannerModal.vuefrontend/composables/use-barcode-detector.tsfrontend/composables/utils.test.tsfrontend/composables/utils.ts
| const assetMatch = sanitized.match(/^\/a\/([a-zA-Z0-9-_]+)/i); | ||
| if (assetMatch) return { entityType: "asset", id: assetMatch[1]! }; | ||
|
|
||
| const itemMatch = sanitized.match(/^\/item\/([a-zA-Z0-9-_]+)/); | ||
| const itemMatch = sanitized.match(/^\/item\/([a-zA-Z0-9-_]+)/i); | ||
| if (itemMatch) return { entityType: "item", id: itemMatch[1]! }; | ||
|
|
||
| const locationMatch = sanitized.match(/^\/location\/([a-zA-Z0-9-_]+)/); | ||
| const locationMatch = sanitized.match(/^\/location\/([a-zA-Z0-9-_]+)/i); | ||
| if (locationMatch) return { entityType: "location", id: locationMatch[1]! }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JavaScript RegExp character classes, does [a-zA-Z0-9-]match characters beyond letters/digits/underscore/hyphen? What is the safest equivalent for exactly alnum ++-?
💡 Result:
In JavaScript regular expressions, the character class [a-zA-Z0-9-] does not match characters beyond letters, digits, underscores, and hyphens, provided the hyphen is placed correctly [1][2][3]. However, if the hyphen is placed between two characters (e.g., [a-z0-9-]), it may be interpreted as a range operator depending on the surrounding characters, which can lead to unexpected behavior [4][5]. The hyphen (-) is a special character inside character classes used to define ranges (e.g., a-z) [6][7]. If it is not placed at the very beginning or the very end of the class, or if it is not escaped with a backslash (-), the regex engine may attempt to create a range between the characters immediately preceding and following it [1][2][8]. For example, in [a-z0-9-], the hyphen is between 9 and , which is generally safe because the ASCII value of 9 is less than the ASCII value of , but it is best practice to avoid ambiguity [6][5]. The safest and most readable equivalent for exactly alphanumeric characters (a-z, A-Z, 0-9), underscores (), and hyphens (-) is: /[\w-]/ In JavaScript, the \w shorthand character class is equivalent to [A-Za-z0-9] [1][2][3]. By placing the hyphen at the end of the class ([\w-]), you ensure it is treated as a literal hyphen without needing to escape it [1][2][8]. If you prefer to be explicit or want to avoid the \w shorthand, the safest explicit pattern is: /[a-zA-Z0-9-]/ In this version, the hyphen is escaped (-), which explicitly tells the regex engine to treat it as a literal character regardless of its position [4][8].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular%5Fexpressions/Cheatsheet
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions/Character_classes
- 3: https://stackoverflow.com/questions/38265411/regular-expression-for-alphanumeric-hyphen-and-underscore-without-any-space
- 4: https://stackoverflow.com/questions/3697202/including-a-hyphen-in-a-regex-character-bracket
- 5: https://stackoverflow.com/questions/34916716/regular-expression-to-match-alphanumeric-hyphen-underscore-and-space-string/34916893
- 6: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Character_class
- 7: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Regex_character_class_escape_in_class_range
- 8: https://stackoverflow.com/questions/9589074/regex-should-hyphens-be-escaped
🏁 Script executed:
# Check if the file exists and read the relevant lines
if [ -f "frontend/composables/use-barcode-detector.ts" ]; then
echo "File exists. Reading lines 85-100:"
sed -n '85,100p' "frontend/composables/use-barcode-detector.ts"
else
echo "File not found. Searching for it..."
find . -name "use-barcode-detector.ts" -type f
fiRepository: sysadminsmedia/homebox
Length of output: 731
Fix regex character class escaping on lines 84 and 89–96 to prevent matching unintended punctuation.
The character class [a-zA-Z0-9-_] treats the hyphen (between 9 and _) as a range operator, matching ASCII 57–95, which includes unwanted punctuation like :;<=>?@[\]^. This affects both the sanitization regex on line 84 and the ID extraction patterns on lines 89–96.
Move the hyphen to the end of the character class or escape it: [A-Za-z0-9_-] or [A-Za-z0-9_\-].
Suggested patch
- const sanitized = pathname.replace(/[^a-zA-Z0-9-_/]/g, "");
+ const sanitized = pathname.replace(/[^A-Za-z0-9_-/]/g, "");
// Case-insensitive so uppercase-encoded codes resolve to the right entity.
- const assetMatch = sanitized.match(/^\/a\/([a-zA-Z0-9-_]+)/i);
+ const assetMatch = sanitized.match(/^\/a\/([A-Za-z0-9_-]+)/i);
- const itemMatch = sanitized.match(/^\/item\/([a-zA-Z0-9-_]+)/i);
+ const itemMatch = sanitized.match(/^\/item\/([A-Za-z0-9_-]+)/i);
- const locationMatch = sanitized.match(/^\/location\/([a-zA-Z0-9-_]+)/i);
+ const locationMatch = sanitized.match(/^\/location\/([A-Za-z0-9_-]+)/i);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/composables/use-barcode-detector.ts` around lines 89 - 96, The regex
character class used to sanitize and extract IDs (the patterns applied to
sanitized and in the assetMatch/itemMatch/locationMatch tests) incorrectly
places the hyphen between 9 and _, creating an unintended range; update those
regexes (the sanitization regex and the three extraction regexes in
useBarcodeDetector/composable where sanitized.match is used) to either escape
the hyphen or move it to the end of the class (e.g., change [a-zA-Z0-9-_] to
[A-Za-z0-9_-] or [A-Za-z0-9_\-]) so IDs only match letters, digits, underscore
and hyphen.
|
Regarding CodeRabbit feedback
|
1 participant
What type of PR is this?
What this PR does / why we need it:
Lets the scanner read protocol-less URLs like
example.com/a/1.Dropping
https://and using uppercase characters fits more data into the smallest Data Matrix / QR codes (C40 mode), so users can pre-print tiny asset stickers.parseScanResult(new, incomposables/utils.ts):scheme://, accept only http(s) with a host. Rejectfile://,mailto:,data:,javascript:, etc./so plain EAN/UPC barcode digits still fall through to the existing product-lookup path.Used by both
ScannerModal.vueandparseHomeboxUrlin the AR scanner. The AR scanner's path regexes also got theiflag so uppercase paths resolve to the right entity type (vue-router already routes static path segments case-insensitively, but the AR scanner classifies the entity via its own regex).Which issue(s) this PR fixes:
None
Testing
20 unit tests in
composables/utils.test.tscovering: full http(s) URLs, mixed-case URLs, protocol-less host+path payloads (with and without port, with and without uppercase), current-page protocol fallback, bare paths and numeric barcodes, protocol-relative URLs, and non-http schemes file://, mailto:, javascript:, data:.