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

js_to_json() is swallowing commas in JS dicts at the end of a line when a line comment ending in } is following #16932

Closed
mhartkorn opened this issue Jul 8, 2018 · 0 comments

Comments

@mhartkorn
Copy link

@mhartkorn mhartkorn commented Jul 8, 2018

Make sure you are using the latest version: run youtube-dl --version and ensure your version is 2018.07.04. If it's not, read this FAQ entry and update. Issues with outdated version will be rejected.

  • I've verified and I assure that I'm running youtube-dl 2018.07.04

Before submitting an issue make sure you have:

  • At least skimmed through the README, most notably the FAQ and BUGS sections
  • Searched the bugtracker for similar issues including closed ones
  • Checked that provided video/audio/playlist URLs (if any) are alive and playable in a browser

What is the purpose of your issue?

  • Bug report (encountered problems with youtube-dl)
  • Site support request (request for adding support for a new site)
  • Feature request (request for a new functionality)
  • Question
  • Other

Description of your issue, suggested solution and other information

As mentioned in PR #16317 by @dstftw js_to_json() has a bug that is removing commas (,) at line endings when a } inside a line comment is following. I debugged this a bit and got to the following minimal reproducible example.

Extracted JavaScript code from website:

{foo: 0, //}
bar : 1}

Result after putting it into js_to_json() (notice the space after 0):

{"foo": 0 
"bar" : 1}

When running it in a debugger, it turns out that fix_kv() (the callback inside js_to_json()) is receiving the , after 0 as a separate match and because of this condition it's removing the comma all together:

if v in ('true', 'false', 'null'):
    return v
elif v.startswith('/*') or v.startswith('//') or v == ',':
    return ""

I cannot fix this by myself because I'm not familiar enough with regex to make an adequate fix. I suspect that there should be a check added to see if the following } (which is apparently interpreted as the termination of the dict) is inside a comment (or the skip group alltogether?).

@mhartkorn mhartkorn mentioned this issue Jul 8, 2018
6 of 9 tasks complete
@mhartkorn mhartkorn changed the title js_to_json() is swallowing commas after the end of an array when a line comment ending in } is following js_to_json() is swallowing commas in JS dicts at the end of a line when a line comment ending in } is following Jul 8, 2018
@mhartkorn mhartkorn closed this Jan 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.