fix(cid): localhost should only be considered as proxy if prefix is c or v.#1289
Conversation
|
Do the unit tests still go through the same code path? |
ea294e7 to
2651400
Compare
|
@cramforce with this current code, i can't 100% guarantee that. i think there could certainly be an indirect use of the im thinking of just removing the changes i made and doing this: if (url.origin.indexOf('http://localhost:') != 0) {
assert(prefix == 'c' || prefix == 'v',
'Unknown path prefix in url %s', url.href);
}in |
2651400 to
fe56bb8
Compare
|
I'm confused by the changeset that GH is showing now. Otherwise it was fine. |
|
This LGTM. What's the |
|
@cramforce i dont think i was confident that the previous code wouldn't be forcing the tests into the not sure what the v/c stands for, @cramforce can you shed some light on that one? |
a641501 to
21563f5
Compare
src/service/cid-impl.js
Outdated
There was a problem hiding this comment.
I think what we discussed was to instead make #isProxyOrigin only return true if the assert would not fail.
21563f5 to
271534f
Compare
271534f to
484067b
Compare
There was a problem hiding this comment.
@cramforce PTAL. changed to check if localhost has prefix c/v
|
LGTM |
fix(cid): localhost should only be considered as proxy if prefix is c or v.
No description provided.