Repository navigation
glslc: escape special characters and reject newlines in depfiles (fixes #1600) - #1608
Open
jdymitarai wants to merge 1 commit into
Open
jdymitarai wants to merge 1 commit into
jdymitarai wants to merge 1 commit into
Conversation
…fixes google#1600) Signed-off-by: jdymitarai <o10040115@gmail.com>
dneto0
approved these changes
Oct 19, 2026
dneto0
left a comment
dneto0
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for the the thorough tests. It made it much easier to review!
| break; | ||
| case ' ': | ||
| case '\t': | ||
| out << "\\ "; |
Collaborator
There was a problem hiding this comment.
Why does a tab not map to an escaped-tab?
This is inconsistent with glslang's escaping logic, which doesn't handle tabs specially.
On the other hand the munge() function is also quite different, and its comment says it's not possible to do a perfect job.
| TEST(DependencyInfoEscapeTest, TabEscapedAsWhitespace) { | ||
| std::string escaped; | ||
| EXPECT_TRUE(EscapeDepPath("path\twith\ttabs.vert", &escaped)); | ||
| EXPECT_EQ("path\\ with\\ tabs.vert", escaped); |
Collaborator
There was a problem hiding this comment.
As I commented in the feature code, this surprises me. But in any case this overall patch improves the situation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When generating dependency files via -M or -MD, glslc was outputting target names, source paths, and included headers without escaping GNU make special characters or validating for unrepresentable newlines. This allowed paths containing dollar signs to execute arbitrary subshells like
$(shell ...)and allowed newlines to inject unintended make rules.Following GCC munge() (
libcpp/mkdeps.cc) and glslangwriteEscapedDepString(), this change escapes$, space, tab,#,:,\,[, and]in dependency targets and prerequisites, while rejecting unrepresentable newline and carriage return characters with a diagnostic error.We have added comprehensive unit tests covering character escaping and newline rejection in
glslc_dependency_info_test, along with end-to-end regression tests inoption_dash_M.py. All 25 glslc and shaderc test suites pass cleanly.