Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 BUG: CRLF line endings in comments causes an error #915

Closed
KianNH opened this issue May 6, 2022 · 2 comments · Fixed by #936 or #935
Closed

🐛 BUG: CRLF line endings in comments causes an error #915

KianNH opened this issue May 6, 2022 · 2 comments · Fixed by #936 or #935
Assignees

Comments

@KianNH
Copy link
Contributor

KianNH commented May 6, 2022

What version of Wrangler are you using?

0.0.30

What operating system are you using?

Windows

Describe the Bug

Comments in wrangler.toml files saved with CRLF line endings cause an error.

Example wrangler.toml

name = "my-worker"
main = "src/index.ts"
compatibility_date = "2022-05-06" # comment
  • When saved with LF line endings, there are no issues.
  • When saved with CRLF line endings, an error is thrown.

Error

 ⛅️ wrangler 0.0.30
--------------------

X [ERROR] X [ERROR] Control characters (codes < 0x1f and 0x7f) are not allowed in comments, use \u000d instead

image

@KianNH KianNH added the bug label May 6, 2022
@KianNH
Copy link
Contributor Author

KianNH commented May 6, 2022

Example reproduction (on Windows) - uses the https://github.com/cloudflare/rustwasm-worker-template/blob/master/wrangler.toml template since it's the one I first encountered the issue on.

git clone https://github.com/cloudflare/rustwasm-worker-template.git
cd rustwasm-worker-template
npx wrangler@0.0.30 dev
PS C:\Users\K\Desktop\rustwasm-worker-template> npx wrangler@0.0.30 dev
 ⛅️ wrangler 0.0.30
--------------------

X [ERROR] X [ERROR] Control characters (codes < 0x1f and 0x7f) are not allowed in comments, use \u000d instead


      C:\Users\K\Desktop\rustwasm-worker-template\wrangler.toml:10:77:
        10 │ command = "cargo install -q worker-build && worker-build --release" #
  required
           ╵                                                                              ^



    If you think this is a bug, please open an issue at:
    https://github.com/cloudflare/wrangler2/issues/new
PS C:\Users\K\Desktop\rustwasm-worker-template> git ls-files --eol
i/lf    w/crlf  attr/                   .gitignore
i/lf    w/crlf  attr/                   CODE_OF_CONDUCT.md
i/lf    w/crlf  attr/                   Cargo.lock
i/lf    w/crlf  attr/                   Cargo.toml
i/lf    w/crlf  attr/                   LICENSE_APACHE
i/lf    w/crlf  attr/                   README.md
i/lf    w/crlf  attr/                   src/lib.rs
i/lf    w/crlf  attr/                   src/utils.rs
i/lf    w/crlf  attr/                   wrangler.toml

@threepointone
Copy link
Contributor

Great catch, looks like an issue with our parser iarna/iarna-toml#33 I'll work on a fix soon.

@threepointone threepointone moved this to Should-have in workers-sdk May 7, 2022
@threepointone threepointone added this to the 2.0 milestone May 7, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue May 9, 2022
The TOML parser that Wrangler uses crashes if there is a Windows line-ending in a comment.
See iarna/iarna-toml#33.

According to the TOML spec, we should be able to normalize line-endings as we see fit.
See https://toml.io/en/v1.0.0#:~:text=normalize%20newline%20to%20whatever%20makes%20sense.

This change normalizes line-endings of TOML strings before parsing to avoid hitting this bug.

Fixes cloudflare#915
@petebacondarwin petebacondarwin self-assigned this May 9, 2022
@petebacondarwin petebacondarwin moved this from Should-have to In Review in workers-sdk May 9, 2022
threepointone pushed a commit that referenced this issue May 9, 2022
The TOML parser that Wrangler uses crashes if there is a Windows line-ending in a comment.
See iarna/iarna-toml#33.

According to the TOML spec, we should be able to normalize line-endings as we see fit.
See https://toml.io/en/v1.0.0#:~:text=normalize%20newline%20to%20whatever%20makes%20sense.

This change normalizes line-endings of TOML strings before parsing to avoid hitting this bug.

Fixes #915
Repository owner moved this from In Review to Done in workers-sdk May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants