You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In process_entity_whitelist, the urls set is updated but never checked against, so duplicates may be added. Shouldn't we check if a url is already in this set before adding it to the list?
Something else that I noticed is that the if prop == res check (https://github.com/mozilla-services/shavar-list-creation/blob/master/lists2safebrowsing.py#L348) happens before the domains corresponding to prop and res are canonicalized. This means that if prop is www.example...com/// and res is www.example.com, www.example.com/?resource=www.example.com will be added to the list. It seems to me that this defeats the purpose of the if prop == res check.
Another observation is that because canonicalization is not applied to query parameters, in case prop is www.example.com and res is www.example...com/// the url added to the list will be www.example.com/?resource=www.example...com///. Is that intentional?
The text was updated successfully, but these errors were encountered:
In
process_entity_whitelist
, theurls
set is updated but never checked against, so duplicates may be added. Shouldn't we check if a url is already in this set before adding it to the list?Something else that I noticed is that the
if prop == res
check (https://github.com/mozilla-services/shavar-list-creation/blob/master/lists2safebrowsing.py#L348) happens before the domains corresponding toprop
andres
are canonicalized. This means that ifprop
iswww.example...com///
andres
iswww.example.com
,www.example.com/?resource=www.example.com
will be added to the list. It seems to me that this defeats the purpose of theif prop == res
check.Another observation is that because canonicalization is not applied to query parameters, in case
prop
iswww.example.com
andres
iswww.example...com///
the url added to the list will bewww.example.com/?resource=www.example...com///
. Is that intentional?The text was updated successfully, but these errors were encountered: