Closed Bug 1600121 Opened 5 years ago Closed 4 years ago

Get rid of check for target path length on target-created

Categories

(Remote Protocol :: Agent, task, P1)

task

Tracking

(firefox73 fixed)

RESOLVED FIXED
Tracking Status
firefox73 --- fixed

People

(Reporter: ato, Assigned: ato)

References

Details

(Whiteboard: [puppeteer-alpha-reserve])

Attachments

(2 files)

When target-created is handled we check explicitly that it’s URL path is not empty:

    this.targets.on("target-created", (eventName, target) => {
      if (!target.path) {
        throw new Error(`Target is missing 'path' attribute: ${target}`);
      }
      this.server.registerPathHandler(target.path, target);
    });

It should be illegal in nsIHttpServer.registerPathHandler to register a path
on an empty string because it cannot later be accessed or referenced, i.e. deleted.

It also blocks bug 1590828 because we cannot error in async code.

Assignee: nobody → ato
Blocks: 1590828
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [puppeteer-alpha-reserve]

It should be illegal to add paths that cannot be handled/accessed
or later referenced. Without a path, it is for example later
impossible to delete the handler.

To address this we return an NS_ERROR_INVALID_ARG when
nsIHttpServer.registerPathHandler is called with an empty string.

As nsIHttpServer.registerPathHandler now checks that the path is
not empty we can drop our pre-check when target-created is handled.

If target.path does happen to be empty an error is printed to the console.

Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40b6d9ae0601
netwerk: error on empty path in nsIHttpServer.registerPathHandler; r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/0342c32685da
remote: rely on nsIHttpServer.registerPathHandler to assert path r=remote-protocol-reviewers,maja_zf,whimboo
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: