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

Fix double encoding of inputs in worker request #23924

Closed
wants to merge 2 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Oct 9, 2024

Since WorkRequest encodes strings as UTF-8, Bazel's internal string encoding of file paths needs to be undone before passing them to the protobuf builder.

Since `WorkRequest` encodes strings as UTF-8, Bazel's internal string encoding of file paths needs to be undone before passing them to the protobuf builder.
@fmeum fmeum requested a review from meisterT October 9, 2024 18:30
@fmeum
Copy link
Collaborator Author

fmeum commented Oct 9, 2024

@bazel-io fork 8.0.0

@github-actions github-actions bot added team-Local-Exec Issues and PRs for the Execution (Local) team awaiting-review PR is awaiting review from an assigned reviewer labels Oct 9, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Oct 9, 2024

@bazel-io fork 7.4.0

@meisterT
Copy link
Member

meisterT commented Oct 9, 2024

How did this not show up as problematic before?

@fmeum
Copy link
Collaborator Author

fmeum commented Oct 10, 2024

How did this not show up as problematic before?

The only worker supplied with Bazel (Javac) doesn't read the field. I don't know whether any worker does. Multiplex sandboxing would probably be a better way to do that.

@tjgq tjgq self-assigned this Oct 22, 2024
@fmeum fmeum requested a review from tjgq October 22, 2024 15:28
@tjgq tjgq added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Oct 28, 2024
@iancha1992
Copy link
Member

@bazel-io fork 7.4.1

@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Oct 28, 2024
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Oct 28, 2024
Since `WorkRequest` encodes strings as UTF-8, Bazel's internal string encoding of file paths needs to be undone before passing them to the protobuf builder.

Closes bazelbuild#23924.

PiperOrigin-RevId: 690737819
Change-Id: I3eb341c05e045c75ac004da57e1f46f3587a6810
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Oct 28, 2024
Since `WorkRequest` encodes strings as UTF-8, Bazel's internal string encoding of file paths needs to be undone before passing them to the protobuf builder.

Closes bazelbuild#23924.

PiperOrigin-RevId: 690737819
Change-Id: I3eb341c05e045c75ac004da57e1f46f3587a6810
iancha1992 pushed a commit that referenced this pull request Oct 29, 2024
Since `WorkRequest` encodes strings as UTF-8, Bazel's internal string
encoding of file paths needs to be undone before passing them to the
protobuf builder.

Closes #23924.

PiperOrigin-RevId: 690737819
Change-Id: I3eb341c05e045c75ac004da57e1f46f3587a6810

Commit
89f798e

Co-authored-by: Fabian Meumertzheim <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Oct 29, 2024
Since `WorkRequest` encodes strings as UTF-8, Bazel's internal string
encoding of file paths needs to be undone before passing them to the
protobuf builder.

Closes #23924.

PiperOrigin-RevId: 690737819
Change-Id: I3eb341c05e045c75ac004da57e1f46f3587a6810

Commit
89f798e

Co-authored-by: Fabian Meumertzheim <[email protected]>
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 7.4.1 RC1. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.4.1rc1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Local-Exec Issues and PRs for the Execution (Local) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants