Skip to content

Commit 6032d64

Browse files
justin808claude
andcommitted
Improve bundle path resolution with secure server bundle locations
Major improvements to bundle_js_file_path logic: **Security & Architecture:** - Server bundles (SSR/RSC) now try secure non-public locations first: - ssr-generated/ (primary) - generated/server-bundles/ (secondary) - Client bundles continue using manifest lookup as primary approach - Prevents exposure of server-side logic in public directories **Priority Order:** - SERVER BUNDLES: secure locations → manifest → legacy public locations - CLIENT BUNDLES: manifest → fallback locations (original behavior) - Fixed priority order (normal case first, edge cases second) **Code Quality:** - Extracted complex method into smaller, focused private methods - Reduced cyclomatic complexity and improved maintainability - Added comprehensive test coverage for all scenarios - Added ssr-generated/ to .gitignore **Backwards Compatibility:** - Legacy public locations still work as fallbacks - Existing client bundle behavior unchanged - Gradual migration path for server bundles This addresses the core architectural issue where server bundles were unnecessarily exposed in public directories while maintaining full compatibility with existing setups. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 89ac6eb commit 6032d64

File tree

3 files changed

+144
-37
lines changed

3 files changed

+144
-37
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,5 +61,8 @@ yalc.lock
6161
# Generated by ROR FS-based Registry
6262
generated
6363

64+
# Server-side rendering generated bundles
65+
ssr-generated
66+
6467
# Claude Code local settings
6568
.claude/settings.local.json

lib/react_on_rails/utils.rb

Lines changed: 69 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -69,38 +69,11 @@ def self.server_bundle_path_is_http?
6969
end
7070

7171
def self.bundle_js_file_path(bundle_name)
72-
# Either:
73-
# 1. Using same bundle for both server and client, so server bundle will be hashed in manifest
74-
# 2. Using a different bundle (different Webpack config), so file is not hashed, and
75-
# bundle_js_path will throw so the default path is used without a hash.
76-
# 3. The third option of having the server bundle hashed and a different configuration than
77-
# the client bundle is not supported for 2 reasons:
78-
# a. The webpack manifest plugin would have a race condition where the same manifest.json
79-
# is edited by both the webpack-dev-server
80-
# b. There is no good reason to hash the server bundle name.
72+
# Priority order depends on bundle type:
73+
# SERVER BUNDLES (normal case): Try secure non-public locations first, then manifest, then legacy
74+
# CLIENT BUNDLES (normal case): Try manifest first, then fallback locations
8175
if ReactOnRails::PackerUtils.using_packer? && bundle_name != "manifest.json"
82-
begin
83-
ReactOnRails::PackerUtils.bundle_js_uri_from_packer(bundle_name)
84-
rescue Shakapacker::Manifest::MissingEntryError
85-
# When manifest lookup fails, try multiple fallback locations:
86-
# 1. Environment-specific path (e.g., public/webpack/test)
87-
# 2. Standard Shakapacker location (public/packs)
88-
# 3. Generated assets path (for legacy setups)
89-
fallback_locations = [
90-
File.join(ReactOnRails::PackerUtils.packer_public_output_path, bundle_name),
91-
File.join("public", "packs", bundle_name),
92-
File.join(generated_assets_full_path, bundle_name)
93-
].uniq
94-
95-
# Return the first location where the bundle file actually exists
96-
fallback_locations.each do |path|
97-
expanded_path = File.expand_path(path)
98-
return expanded_path if File.exist?(expanded_path)
99-
end
100-
101-
# If none exist, return the environment-specific path (original behavior)
102-
File.expand_path(fallback_locations.first)
103-
end
76+
bundle_js_file_path_with_packer(bundle_name)
10477
else
10578
# Default to the non-hashed name in the specified output directory, which, for legacy
10679
# React on Rails, this is the output directory picked up by the asset pipeline.
@@ -109,6 +82,71 @@ def self.bundle_js_file_path(bundle_name)
10982
end
11083
end
11184

85+
def self.bundle_js_file_path_with_packer(bundle_name)
86+
is_server_bundle = server_bundle?(bundle_name)
87+
88+
if is_server_bundle
89+
# NORMAL CASE for server bundles: Try secure non-public locations first
90+
secure_path = try_secure_server_locations(bundle_name)
91+
return secure_path if secure_path
92+
end
93+
94+
# For client bundles OR server bundle fallback: Try manifest lookup
95+
begin
96+
ReactOnRails::PackerUtils.bundle_js_uri_from_packer(bundle_name)
97+
rescue Shakapacker::Manifest::MissingEntryError
98+
handle_missing_manifest_entry(bundle_name, is_server_bundle)
99+
end
100+
end
101+
102+
def self.server_bundle?(bundle_name)
103+
bundle_name == ReactOnRails.configuration.server_bundle_js_file ||
104+
bundle_name == ReactOnRails.configuration.rsc_bundle_js_file
105+
end
106+
107+
def self.try_secure_server_locations(bundle_name)
108+
secure_server_locations = [
109+
File.join("ssr-generated", bundle_name),
110+
File.join("generated", "server-bundles", bundle_name)
111+
]
112+
113+
secure_server_locations.each do |path|
114+
expanded_path = File.expand_path(path)
115+
return expanded_path if File.exist?(expanded_path)
116+
end
117+
118+
nil
119+
end
120+
121+
def self.handle_missing_manifest_entry(bundle_name, is_server_bundle)
122+
# When manifest lookup fails, try multiple fallback locations:
123+
# 1. Environment-specific path (e.g., public/webpack/test)
124+
# 2. Standard Shakapacker location (public/packs)
125+
# 3. Generated assets path (for legacy setups)
126+
fallback_locations = [
127+
File.join(ReactOnRails::PackerUtils.packer_public_output_path, bundle_name),
128+
File.join("public", "packs", bundle_name),
129+
File.join(generated_assets_full_path, bundle_name)
130+
].uniq
131+
132+
# Return the first location where the bundle file actually exists
133+
fallback_locations.each do |path|
134+
expanded_path = File.expand_path(path)
135+
return expanded_path if File.exist?(expanded_path)
136+
end
137+
138+
# If none exist, return appropriate default based on bundle type
139+
if is_server_bundle
140+
# For server bundles, prefer secure location as final fallback
141+
File.expand_path(File.join("ssr-generated", bundle_name))
142+
else
143+
# For client bundles, use environment-specific path (original behavior)
144+
File.expand_path(fallback_locations.first)
145+
end
146+
end
147+
private_class_method :bundle_js_file_path_with_packer, :server_bundle?, :try_secure_server_locations,
148+
:handle_missing_manifest_entry
149+
112150
def self.server_bundle_js_file_path
113151
return @server_bundle_path if @server_bundle_path && !Rails.env.development?
114152

spec/react_on_rails/utils_spec.rb

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,14 @@ def mock_dev_server_running
103103
end
104104

105105
describe ".bundle_js_file_path" do
106+
before do
107+
# Mock configuration calls to avoid server bundle detection for regular client bundles
108+
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
109+
.and_return("server-bundle.js")
110+
allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
111+
.and_return("rsc-bundle.js")
112+
end
113+
106114
subject do
107115
described_class.bundle_js_file_path("webpack-bundle.js")
108116
end
@@ -158,6 +166,64 @@ def mock_dev_server_running
158166
expect(result).to eq(File.expand_path(env_specific_path))
159167
end
160168
end
169+
170+
context "with server bundle (SSR/RSC) file not in manifest" do
171+
let(:server_bundle_name) { "server-bundle.js" }
172+
let(:ssr_generated_path) { File.expand_path(File.join("ssr-generated", server_bundle_name)) }
173+
let(:generated_server_path) do
174+
File.expand_path(File.join("generated", "server-bundles", server_bundle_name))
175+
end
176+
177+
before do
178+
mock_missing_manifest_entry(server_bundle_name)
179+
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_js_file")
180+
.and_return(server_bundle_name)
181+
end
182+
183+
it "tries secure locations first for server bundles" do
184+
allow(File).to receive(:exist?).and_call_original
185+
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
186+
187+
result = described_class.bundle_js_file_path(server_bundle_name)
188+
expect(result).to eq(ssr_generated_path)
189+
end
190+
191+
it "tries generated/server-bundles as second secure location" do
192+
allow(File).to receive(:exist?).and_call_original
193+
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(false)
194+
allow(File).to receive(:exist?).with(generated_server_path).and_return(true)
195+
196+
result = described_class.bundle_js_file_path(server_bundle_name)
197+
expect(result).to eq(generated_server_path)
198+
end
199+
200+
it "falls back to ssr-generated location when no bundle exists anywhere" do
201+
allow(File).to receive(:exist?).and_call_original
202+
allow(File).to receive(:exist?).and_return(false)
203+
204+
result = described_class.bundle_js_file_path(server_bundle_name)
205+
expect(result).to eq(ssr_generated_path)
206+
end
207+
end
208+
209+
context "with RSC bundle file not in manifest" do
210+
let(:rsc_bundle_name) { "rsc-bundle.js" }
211+
let(:ssr_generated_path) { File.expand_path(File.join("ssr-generated", rsc_bundle_name)) }
212+
213+
before do
214+
mock_missing_manifest_entry(rsc_bundle_name)
215+
allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
216+
.and_return(rsc_bundle_name)
217+
end
218+
219+
it "treats RSC bundles as server bundles and tries secure locations first" do
220+
allow(File).to receive(:exist?).and_call_original
221+
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
222+
223+
result = described_class.bundle_js_file_path(rsc_bundle_name)
224+
expect(result).to eq(ssr_generated_path)
225+
end
226+
end
161227
end
162228
end
163229

@@ -204,14 +270,14 @@ def mock_dev_server_running
204270
include_context "with #{packer_type} enabled"
205271

206272
context "with server file not in manifest", packer_type.to_sym do
207-
it "returns the unhashed server path" do
273+
it "returns the secure ssr-generated path for server bundles" do
208274
server_bundle_name = "server-bundle.js"
209275
mock_bundle_configs(server_bundle_name: server_bundle_name)
210276
mock_missing_manifest_entry(server_bundle_name)
211277

212278
path = described_class.server_bundle_js_file_path
213279

214-
expect(path).to end_with("public/webpack/development/#{server_bundle_name}")
280+
expect(path).to end_with("ssr-generated/#{server_bundle_name}")
215281
end
216282

217283
context "with bundle file existing in standard location but not environment-specific location" do
@@ -235,7 +301,7 @@ def mock_dev_server_running
235301
end
236302

237303
context "with bundle file not existing in any fallback location" do
238-
it "returns the environment-specific path as final fallback" do
304+
it "returns the secure ssr-generated path as final fallback for server bundles" do
239305
server_bundle_name = "server-bundle.js"
240306
mock_bundle_configs(server_bundle_name: server_bundle_name)
241307
mock_missing_manifest_entry(server_bundle_name)
@@ -246,7 +312,7 @@ def mock_dev_server_running
246312

247313
path = described_class.server_bundle_js_file_path
248314

249-
expect(path).to end_with("public/webpack/development/#{server_bundle_name}")
315+
expect(path).to end_with("ssr-generated/#{server_bundle_name}")
250316
end
251317
end
252318
end
@@ -303,14 +369,14 @@ def mock_dev_server_running
303369
include_context "with #{packer_type} enabled"
304370

305371
context "with server file not in manifest", packer_type.to_sym do
306-
it "returns the unhashed server path" do
372+
it "returns the secure ssr-generated path for RSC bundles" do
307373
server_bundle_name = "rsc-bundle.js"
308374
mock_bundle_configs(rsc_bundle_name: server_bundle_name)
309375
mock_missing_manifest_entry(server_bundle_name)
310376

311377
path = described_class.rsc_bundle_js_file_path
312378

313-
expect(path).to end_with("public/webpack/development/#{server_bundle_name}")
379+
expect(path).to end_with("ssr-generated/#{server_bundle_name}")
314380
end
315381
end
316382

0 commit comments

Comments
 (0)