Skip to content

structuredClone / atob / btoa should throw a TypeError without arguments #41450

Closed
#41651
#41478
Closed
structuredClone / atob / btoa should throw a TypeError without arguments#41450
#41651
#41478
Labels
bufferIssues and PRs related to the buffer subsystem.confirmed-bugIssues and PRs for confirmed bugs.errorsIssues and PRs related to JavaScript errors originating in Node.js core.

Description

@zloirock
zloirock
opened on Jan 9, 2022

Version

17.3.0

Platform

MacOS 12.1

Subsystem

global / buffer

What steps will reproduce the bug?

structuredClone(); // => undefined, should be a TypeError
atob(); // => 'ºw^~)Þ', should be a TypeError
btoa(); // => 'dW5kZWZpbmVk', should be a TypeError

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

No response

What do you see instead?

^

Additional information

It's an inconsistency with web standards.

Activity

  1. DerekNonGeneric commented on Jan 9, 2022

    @DerekNonGeneric
    DerekNonGeneric
    on Jan 9, 2022
    Contributor

    @zloirock, thanks for opening this issue. As far as I understand, these utility methods evolved w/o any standard or official specification. The nearest I can find is the link below.

    https://html.spec.whatwg.org/multipage/webappapis.html#atob

    By any chance, would you be able to link a more complete specification or should we derive this specification from current web browser behavior?

  2. zloirock
    changed the title [-]atob / btoa should throw a TypeError without arguments[/-] [+]structuredClone / atob / btoa should throw a TypeError without arguments[/+] on Jan 9, 2022

  3. zloirock commented on Jan 9, 2022

    @zloirock
    zloirock
    on Jan 9, 2022
    Author

    @DerekNonGeneric the same with structuredClone. I'm not an expert in Web IDL, but looks like here specified that they should take a required argument, so they should throw an error if the argument is missed. This is the behavior of all actual browsers, so I think that's interpreted correctly.

  4. zloirock commented on Jan 9, 2022

    @zloirock
    zloirock
    on Jan 9, 2022
    Author

    For example, similarly, in the URL spec we could see that URLSearchParams#append should accept 2 params - and if not, that throws an error everywhere - in Node and all browsers:

    image

    And the error is not specified directly.

  5. VoltrexKeyva commented on Jan 9, 2022

    @VoltrexKeyva
    VoltrexKeyva
    on Jan 9, 2022
    Contributor

    As far as I understand, the WHATWG spec for both of the atob() and btoa() functions/methods states that a required argument must be taken and must be a value with the primitive type of string.

    As for the structuredClone() function, the value can be anything other than specified C++ natives; so I guess it's already giving out the intended output as passing nothing falls back to the undefined value, which it's structured clone is undefined, we could also require the user to pass a value regardless of the fallback behavior by checking the amount of arguments they've passed to the function, but I'm -0 on that.

  6. aduh95 commented on Jan 9, 2022

    @aduh95
    aduh95
    on Jan 9, 2022
    Contributor

    Spec for atob and btoa: https://html.spec.whatwg.org/multipage/webappapis.html#dom-atob-dev

    • Firefox: TypeError: Window.atob: At least 1 argument required, but only 0 passed
    • Safari: TypeError: Not enough arguments
    • Chromium: TypeError: Failed to execute 'atob' on 'Window': 1 argument required, but only 0 present.
    • Deno: DOMException: Failed to decode base64.

    Spec for structuredClone: https://html.spec.whatwg.org/multipage/structured-data.html#dom-structuredclone

    • Firefox: TypeError: Window.structuredClone: At least 1 argument required, but only 0 passed
    • Safari: TypeError: Not enough arguments
    • Chromium: TypeError: Failed to execute 'structuredClone' on 'Window': 1 argument required, but only 0 present.
    • Deno: TypeError: Failed to execute 'structuredClone': 1 argument required, but only 0 present.

    It looks like Node.js indeed not aligned with the rest of the ecosystem.

  7. targos commented on Jan 10, 2022

    @targos
    targos
    on Jan 10, 2022
    Member

    I think this is the part of Web IDL that defines what should happen: https://webidl.spec.whatwg.org/#es-overloads

    If there is no valid overload for the type of value passed in here, then we throw a TypeError.

  8. benjamingr commented on Jan 11, 2022

    @benjamingr
    benjamingr
    on Jan 11, 2022
    Member

    Sure I'll open a quick PR that throws ERR_INVALID_ARG_TYPE and adds a test good catch

  9. benjamingr commented on Jan 11, 2022

    @benjamingr
    benjamingr
    on Jan 11, 2022
    Member

    I can also add a fix for structuredClone to the PR but I'd rather make a good-first-issue out of it if everyone is OK with it given how straightforward it is maybe?

  10. DerekNonGeneric commented on Jan 11, 2022

    @DerekNonGeneric
    DerekNonGeneric
    on Jan 11, 2022
    Contributor

    @benjamingr, do you think it would be best to add a new ERR_INVALID_ARGS_NUMBER code first, or do you plan to go w/ ERR_MISSING_ARGS? Without the name of this missing argument, it might be tough to determine what to provide to the error for the message text.

    If you want me to open a PR to include this ERR_INVALID_ARGS_NUMBER first, it might be best since adding a whole new error code seems a bit much for a GFI, and it can be tricky.

  11. benjamingr commented on Jan 11, 2022

    @benjamingr
    benjamingr
    on Jan 11, 2022
    Member

    @DerekNonGeneric I think it's important to understand what .code is used for and its importance and I don't think it matters in this case. I just think it's important we should raise a TypeError and atob is for browser compatibility anyway so I doubt anyone is checking .code on anything atob related.

    That said, if you feel strongly about this feel free to push whatever code you want on that branch or ask that I change it to ERR_MISSING_ARGS both are fine by me :)

  12. gioragutt commented on Jan 22, 2022

    @gioragutt
    gioragutt
    on Jan 22, 2022
    Contributor

    I'll be happy to take the structuredClone fix. What would be the error of choice in this case? ERR_MISSING_OPTION('value') or ERR_INVALID_ARG_TYPE('value', [...'everything specified in #1'], value)?

  13. benjamingr commented on Jan 22, 2022

    @benjamingr
    benjamingr
    on Jan 22, 2022
    Member

    ERR_MISSING_ARGS? I honestly just care that it's a TypeError and is spec compliant :)

  14. gioragutt commented on Jan 22, 2022

    @gioragutt
    gioragutt
    on Jan 22, 2022
    Contributor

    Oh right, ERR_MISSING_OPTION sounds like an error for a missing key in an options object, ERR_MISSING_ARGS makes more sense 👍🏻

  15. nodejs-github-bot
    closed this as completedin #41651on Jan 24, 2022
  16. nodejs-github-bot
    added a commit that references this issue on Jan 24, 2022

    lib: throw error in structuedClone when no arguments are passed

    b27ae24

  17. benjamingr commented on Jan 24, 2022

    @benjamingr
    benjamingr
    on Jan 24, 2022
    Member

    Only a subset was fixed in the PR - so reopening.

  18. benjamingr
    reopened this on Jan 24, 2022
  19. nodejs-github-bot
    closed this as completedin #41478on Feb 3, 2022
  20. nodejs-github-bot
    added a commit that references this issue on Feb 3, 2022

    buffer: fix atob/btoa no-arg case

    9a8fa71
  21. ruyadorno
    added 2 commits that reference this issue on Feb 8, 2022

    lib: throw error in structuedClone when no arguments are passed

    8ab0540

    buffer: fix atob/btoa no-arg case

    6064b1d
  22. danielleadams
    added 2 commits that reference this issue on Mar 2, 2022

    buffer: fix atob/btoa no-arg case

    bba5d4f

    buffer: fix atob/btoa no-arg case

    1af4ce9
  23. danielleadams
    added a commit that references this issue on Mar 14, 2022

    buffer: fix atob/btoa no-arg case

    14bb6f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bufferIssues and PRs related to the buffer subsystem.confirmed-bugIssues and PRs for confirmed bugs.errorsIssues and PRs related to JavaScript errors originating in Node.js core.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions