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

A more robust parsing of the manifest #89

Merged
merged 3 commits into from
Jun 16, 2017

Conversation

rekka
Copy link
Contributor

@rekka rekka commented Jun 11, 2017

This partially addresses #74 without actually changing the manifest format.

Now the lines in the manifest are split from the end (rsplitn) into exactly three pieces and only on
spaces (' ').

This allows filenames to have spaces in their names.

A sanity check has been added to not insert files with newline characters in their names. (I guess to prevent the potential to doctor a manifest line by crafting a file name).

Question: Is it worth inserting files with names that are not valid unicode, i.e. use to_string_lossy? This record is then useless anyway because it will not match the original file.

Split lines from the end into exactly three piecess and split only on
spaces (' ').

This allows filenames to have spaces in their names.
@pkgw
Copy link
Collaborator

pkgw commented Jun 13, 2017

Nice! Yes, if we're trying to be thorough here, I guess we should also not try to record filenames that are invalid UTF-8, although I don't believe that TeX programs are able to construct such filenames.

@rekka
Copy link
Contributor Author

rekka commented Jun 13, 2017

Thanks for having a look. Honestly, I have not been able to figure out how to construct a filename with a newline either to test the check.

@pkgw
Copy link
Collaborator

pkgw commented Jun 14, 2017

I think it might be possible with a ^^J escape or something, but I am OK with not having a test to cover that particular codepath.

If you can add a change that checks the result of to_string_lossy in order to infer whether the path was valid UTF-8, I'd call this good to merge.

@rekka
Copy link
Contributor Author

rekka commented Jun 14, 2017

OK, done. Thanks!

^^J indeed works, that's a neat trick. And the file does not get inserted, as expected.

@pkgw pkgw merged commit 032f6ff into tectonic-typesetting:master Jun 16, 2017
@pkgw
Copy link
Collaborator

pkgw commented Jun 16, 2017

Thanks for the improvement!

@rekka rekka deleted the feat-robust-manifest branch August 24, 2018 03:58
Mrmaxmeier pushed a commit to Mrmaxmeier/tectonic that referenced this pull request Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants