-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Homepage url validation for extension listing #12389
Conversation
if (context.metadata.homepage) { | ||
var parsed = PathUtils.parseUrl(context.metadata.homepage); | ||
// Check if the homepage refers to a local resource | ||
if (parsed.protocol.trim().toLowerCase() === "file:") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@swmitra If have something like C:\Windows\System32\notepad.exe
, will parsed.protocol
list it under file:
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch as always:+1: @nethip .
You are right. It gives us "C:" as protocol 😄
Seems like we can use browser itself to give us all the relevant details of a url instead of relying on the text processing done by path-utils.
…ra/HomePageEntryValidation # Conflicts: # src/extensibility/ExtensionManagerView.js
@nethip Local resources without explicit mention of protocol should be checked now with the changes. Only http/https resources will get skipped. |
@swmitra There seems to be some undesired changes, now part of the PR. Could you check and revert them? |
|
||
// We can't rely on path-utils because of known problems with protocol identification | ||
// Falling back to Browsers protocol identification mechanism | ||
var tmpLink = document.createElement('a'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to create an 'a' element everytime. You can resuse it? The GC of V8 is memory hungry, so will never know when this will be GC'ed.
Travis is failing because of the API limitation. I have manually run |
This PR adds some extra validation on homepage entry in extension listing under ExtensionManager view to make sure local binaries are not getting listed as homepage url.
Ping @nethip @abose @peterflynn for review.