Skip to content

Preserve literal plus signs when decoding asset request paths - #850

Closed
OskarEichler wants to merge 1 commit into
rails:mainfrom
OskarEichler:codex/server-plus
Closed

Preserve literal plus signs when decoding asset request paths#850
OskarEichler wants to merge 1 commit into
rails:mainfrom
OskarEichler:codex/server-plus

Conversation

@OskarEichler

Copy link
Copy Markdown

Summary

The asset server decodes request paths as URI paths: a literal + stays a plus sign, while %20 denotes a space.

Use path decoding instead of form/query decoding. Preserve the previous UTF-8 output encoding explicitly because Rack PATH_INFO can be binary; without that conversion, existing Unicode/invalid-encoding tests detect a regression. Literal/encoded plus signs, spaces, Unicode, GET/HEAD and invalid UTF-8 were checked. The initial encoding regression was corrected before publication.

Reproduction

With require "sprockets"; require "tmpdir":

Dir.mktmpdir do |dir|
  File.write(File.join(dir, "plus+name.js"), "var value=1;")
  env = Sprockets::Environment.new(dir); env.append_path(dir)
  p env.call({"REQUEST_METHOD"=>"GET", "PATH_INFO"=>"/plus+name.js".b}).first
  # 200; previously 404 because + was decoded as a space.
end

Verification

  • Isolated patch: unchanged bundle exec rake test and bundle exec rake test_isolated each pass 913 tests / 4,109 assertions / zero failures/errors / four skips, on Ruby 3.2.11 and 4.0.6 with Rack 3.2.7.
  • 32 focused external checks pass on each Ruby. Baseline reproduces the defect. All Ruby commands used rbenv.
  • Full RuboCop: 133 files, zero offenses. Changed Ruby files pass syntax checks on both Rubies; git diff --check passes.
  • Gem build and content comparison preserve all 84 package paths, version 4.4.1, Ruby >=2.5 and dependency metadata. Only README, CHANGELOG and the relevant runtime files differ.
  • Rack 2.2.24 / Ruby 3.2.11: focused checks pass. Full existing suite has the same preexisting conditional-ETag fixture error on baseline and patch (HTTP_IF_NONE_MATCH is nil at test_server.rb:94); not represented as a passing suite.

Compatibility and limitations

Breaking changes: no intended change to valid supported behavior; the incorrect behavior described above changes. No new dependencies or version upgrades. Nested configuration ownership, unrelated cache invalidation and HTTP behavior are outside this focused patch.

Based on installed release/current upstream main 49d9f20c00ac7d9a7ee57db058961edcccd86699. This is an upstream-only contribution; no personal fork is being adopted in the application. Older Ruby/Linux/JRuby verification is left to upstream CI, not claimed locally. No checked-in tests were added or edited under the requesting repository's no-test-edit policy; focused reproductions ran externally. Prepared with AI assistance. This does not claim exhaustive Sprockets coverage.

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.

2 participants