fix: network filter ending with right hat recognized as regex

[seia-soto]

fixes #3694

---

Summary

This fixes a network filter ending with a hat sign (hostname^) not to be resolved as a regexp filter.

Internals

The NETWORK_FILTER_MASK.isRegex is being set at the following point. The filter has a legit length then checkIsRegex does false positives.

packages/adblocker/src/filters/network.ts

if (filterIndexEnd - filterIndexStart > 0) {
  filter = line.slice(filterIndexStart, filterIndexEnd).toLowerCase();

  mask = setNetworkMask(mask, NETWORK_FILTER_MASK.isUnicode, hasUnicode(filter));
  if (getBit(mask, NETWORK_FILTER_MASK.isRegex) === false) {
    mask = setNetworkMask(
      mask,
      NETWORK_FILTER_MASK.isRegex,
      checkIsRegex(filter, 0, filter.length),
    );
  }
}

checkIsRegex checks if the filter string has a hat sign or asterisk sign. However, this logic also triggers hostname^ pattern to be checked.

/**
 * Check if the sub-string contained between the indices start and end is a
 * regex filter (it contains a '*' or '^' char).
 */
function checkIsRegex(filter: string, start: number, end: number): boolean {
  const indexOfSeparator = filter.indexOf('^', start);
  if (indexOfSeparator !== -1 && indexOfSeparator < end) {
    return true;
  }

  const indexOfWildcard = filter.indexOf('*', start);
  return indexOfWildcard !== -1 && indexOfWildcard < end;
}

Suggested changes

I added a proper hostname checker to validate if the filter is a valid hostname in the checkIsRegex function. The suggested changes check if the filter string has a trailing hat because if a hat sign in other locations does present, it does mean it's often used as a regex symbol.

/**
 * Check if the sub-string contained between the indices start and end is a
 * regex filter (it contains a '*' or '^' char).
 */
function checkIsRegex(filter: string, start: number, end: number): boolean {
  // Check if the filter has a trailing hat without pathes (mostly for hostname) and then trim it
  if (filter.charCodeAt(end - 1) === 94 /* '^' */ && checkIsHostname(filter, start, end - 1)) {
    return false;
  }

  const indexOfSeparator = filter.indexOf('^', start);
  if (indexOfSeparator !== -1 && indexOfSeparator < end) {
    return true;
  }

  const indexOfWildcard = filter.indexOf('*', start);
  return indexOfWildcard !== -1 && indexOfWildcard < end;
}

Also some cases including glob pattern need to be judged. For example, hostname*^ pattern should be handled as regex because it includes glob pattern. Therefore, checking the trailing hat is preferred in this case.

[remusao]

Is this related to the fix of filters ending with ^?

[seia-soto]

I think this is a kind of optimisation. There's a test relevant to this: *bar^. Also see https://ghostery.slack.com/archives/C06JX8NJVMF/p1712045816656759

[remusao]

I think there might be issues with this change. For example the following three cases return false but I would expect all of them to return true instead:

const { Request, f } = require("@cliqz/adblocker");

console.log(f`foo^`.match(Request.fromRawDetails({
  url: "http://foo.com/bar",
  type: "script"
})));

console.log(f`foo^`.match(Request.fromRawDetails({
  url: "http://example.com/foo/bar",
  type: "script"
})));

console.log(f`foo^`.match(Request.fromRawDetails({
  url: "http://foo.com/foo/bar",
  type: "script"
})));

[seia-soto]

The transformation to regexp filter in the parsing logic is intended, Closing.

/**
 * Compiles a filter pattern to a regex. This is only performed *lazily* for
 * filters containing at least a * or ^ symbol. Because Regexes are expansive,
 * we try to convert some patterns to plain filters.
 */
function compileRegex(
  filter: string,
  isLeftAnchor: boolean,
  isRightAnchor: boolean,
  isFullRegex: boolean,
): RegExp

In the future, especially for optimisation, I think an update to matching mechanism should be a better path.