Deal with localhost on implicit values - #478
Merged
Merged
Conversation
I've expanded the scope of this a little to include literal IP addresses as well, after I realized that we weren't able to catch a bunch of problematic names through their use of IP addresses. This is because of awkward things like `[::1]`, but also the wide array of ways of spelling loopback as an IPv4 address. There are also a bunch of non-unique IP addresses that could end up being used to poison things. The check in code is pretty small, but it's not necessarily obvious. The knock-on effect was that it didn't make sense to use SyntaxError for the case where implicit inputs are on loopback. I switched all of the error markings to NotAllowed instead. Closes #456.
apasel422
requested changes
Aug 12, 2026
apasel422
left a comment
apasel422
left a comment
Collaborator
There was a problem hiding this comment.
Can we add a couple of new test cases to https://github.com/w3c/attribution/blob/main/impl/e2e-tests/save-impression-localhost.json and https://github.com/w3c/attribution/blob/main/impl/e2e-tests/measure-conversion-localhost.json reflecting the new behavior? Ideally these would cover the implicit inputs in general, and specifically the IP address and .localhost suffix stuff.
Co-authored-by: Andrew Paseltiner <apaseltiner@google.com>
apasel422
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(This sat on a branch for a whole month. Mostly because I'm not sure if this is the right sort of check, because it expands in a very awkward way.)
I've expanded the scope of this a little to include literal IP addresses as well, after I realized that we weren't able to catch a bunch of problematic names through their use of IP addresses. This is because of awkward things like
[::1], but also the wide array of ways of spelling loopback as an IPv4 address. There are also a bunch of non-unique IP addresses that could end up being used to poison things.The check in code is pretty small, but it's not necessarily obvious.
The knock-on effect was that it didn't make sense to use SyntaxError for the case where implicit inputs are on loopback. I switched all of the error markings to NotAllowed instead.
Closes #456.
Preview | Diff