Skip to content

fix authority normalization#7

Merged
jdesrosiers merged 3 commits intohyperjump-io:mainfrom
srivastava-diya:main
Feb 23, 2026
Merged

fix authority normalization#7
jdesrosiers merged 3 commits intohyperjump-io:mainfrom
srivastava-diya:main

Conversation

@srivastava-diya
Copy link
Contributor

Description

The normalization process was lowercasing the entire authority component, which incorrectly lowercased the userinfo part.

FIXES : #5

Changes

  • Updated resolveReference to copy userinfo, host, and port when the base authority is used.
  • Updated composeIdentifier to build the authority string from components, lowercasing only the host.

Copy link
Contributor

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Oops, I approved and then realized that there are no tests. Please add a test or two showing the intended behavior.

@srivastava-diya
Copy link
Contributor Author

srivastava-diya commented Feb 20, 2026

Oops, I approved and then realized that there are no tests. Please add a test or two showing the intended behavior.

Hey @jdesrosiers i was wondering where should i place the test....should i place it in resolve-reference.test.js or somewhere else?

@jdesrosiers
Copy link
Contributor

Yeah, resolve-references makes sense. There isn't a specific suite for normalization because URIs are normalized when they're resolved so those tests effectively cover normalization as well. (Not necessarily the best testing approach, but it's good enough for this.)

Copy link
Contributor

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

No, just add to the list of resolution tests. Notice the last five tests are all testing normalization. Do something like that.

@srivastava-diya
Copy link
Contributor Author

No, just add to the list of resolution tests. Notice the last five tests are all testing normalization. Do something like that.

my bad, I've removed the standalone test and added it to the resolveTests array.

Copy link
Contributor

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Thank you!

@jdesrosiers jdesrosiers merged commit 9d449c4 into hyperjump-io:main Feb 23, 2026
1 check passed
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.

Bug: Only scheme and host are case insensitive

2 participants