Handle empty response file name in command line parser#3999
Closed
LeSingh1 wants to merge 1 commit into
Closed
Conversation
When tsgo is invoked with a bare '@' argument, the command line parser called parseResponseFile with an empty file name, which reached the vfs layer and panicked with 'vfs: path "" is not absolute' from RootLength. Short-circuit in parseResponseFile when the file name is empty and report the existing Cannot_read_file_0 diagnostic, matching the behavior for a non-existent response file. Fixes microsoft#3758
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a CLI crash in the tsgo command-line parser when a response file argument is provided as a bare @ by short-circuiting response-file parsing and emitting Cannot_read_file_0 instead of reaching the VFS layer panic.
Changes:
- Add an early check in
parseResponseFilefor an empty response file name and reportCannot_read_file_0rather than calling intovfs.FS.ReadFile.
Comments suppressed due to low confidence (1)
internal/tsoptions/commandlineparser.go:169
- Please add a regression test for the
@-only argument so we verify this path reportsCannot_read_file_0instead of panicking (there are existing command line parser tests ininternal/tsoptions/commandlineparser_test.go). This repo’s guidelines expect at least one minimal test case for bug fixes.
if fileName == "" {
p.errors = append(p.errors, ast.NewCompilerDiagnostic(diagnostics.Cannot_read_file_0, fileName))
return
}
| if fileName == "" { | ||
| p.errors = append(p.errors, ast.NewCompilerDiagnostic(diagnostics.Cannot_read_file_0, fileName)) | ||
| return | ||
| } |
Member
|
The right fix is in #3859; the issue is wider than empty strings |
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
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.
Fixes #3758.
Running tsgo with a bare
@argument crashes with a panic in the vfs layer:The crash comes from
parseStringstaking the suffix after@(which is empty) and passing it toparseResponseFile, which then asks the vfs to read the empty path.RootLengthin internal/vfs/internal panics on a non-absolute path, so the process exits before any diagnostic is reported.This change short-circuits at the top of
parseResponseFilewhen the file name is empty and reports the existingCannot_read_file_0diagnostic instead, which is what users would get for any other unreadable response file.The change is four lines in internal/tsoptions/commandlineparser.go. I do not have a Go toolchain on this machine, so I have not run
go test ./...orhereby testlocally. Happy to add a baseline test once a maintainer confirms the approach, or if pointed at the right scenario harness.