Skip to content

Commit

Permalink
Remove requirement for presence of type with redirect= option
Browse files Browse the repository at this point in the history 
Related issue:
- #3590

Since the `redirect=` option was refactored into a modifier
filter, presence of a type (`script`, `xhr`, etc.) is no
longer a requirement.
  • Loading branch information
@gorhill
gorhill committed Nov 28, 2020
1 parent f75040a commit c6d0204
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 10 deletions.
4 changes: 1 addition & 3 deletions docs/tests/static-filtering-parser-checklist.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ a*

! valid options
$script,redirect=noop.js
*$redirect=noop.js
*$empty
*$xhr,empty
*$xhr,redirect=empty
Expand Down Expand Up @@ -91,9 +92,6 @@ $
! bad regex
/(abc|def/$xhr

! can't redirect without type (except to `empty`)
*$redirect=noop.js

! non-redirectable types
*$beacon,redirect-rule=empty
*$ping,redirect-rule=empty
Expand Down
9 changes: 2 additions & 7 deletions src/js/static-filtering-parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2383,18 +2383,13 @@ const NetOptionsIterator = class {
}
}
}
// `redirect=`: requires at least one single redirectable type
// `redirect=`: can't redirect non-redirectable types
{
let i = this.tokenPos[OPTTokenRedirect];
if ( i === -1 ) {
i = this.tokenPos[OPTTokenRedirectRule];
}
if (
i !== -1 && (
hasNoBits(allBits, OPTRedirectableType) ||
hasBits(allBits, OPTNonRedirectableType)
)
) {
if ( i !== -1 && hasBits(allBits, OPTNonRedirectableType) ) {
optSlices[i] = OPTTokenInvalid;
if ( this.interactive ) {
this.parser.errorSlices(optSlices[i+1], optSlices[i+5]);
Expand Down

5 comments on commit c6d0204

@gwarser
Copy link
Contributor

Choose a reason for hiding this comment

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

So now for example noop.js redirect will not be applied to image requests automatically? Or all requests despite of their type will be redirected to noop.js application/javascript?

@gorhill
Copy link
Owner Author

@gorhill gorhill commented on c6d0204 Dec 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The mime is not checked when redirecting to a web-accessible-resource (WAR), this not a change, this has been like this ever since I introduced redirecting to WAR, I made the assumption that the browser is able to sniff the mime from the WAR. When redirecting to a data: URI, the mime matters and no redirection will occur if the mime does not match the type of the request.

@gwarser
Copy link
Contributor

@gwarser gwarser commented on c6d0204 Dec 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

At least in Chrome, headers are not very expressive:

image

I'm thinking about this because you did not included image type here: uBlockOrigin/uAssets@8c97149 and it was broken in stable build.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

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

it was broken in stable build.

I don't understand why, I will need to investigate. There is no change I can think of in dev build which would explain this.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok I forgot, I relaxed the parser requirement in dev build, so it's the parser in stable which prevented the filter from being valid.

Please sign in to comment.