Skip to content

Commit 907fbee

Browse files
committed
Set Cache-Control only for streamed responses
The pull request ruby-grape#1520 introduced a regression that always caused the `Cache-Control` HTTP header to be set to `no-cache`, even if the response wasn't a stream. To fix this, we only set HTTP headers if there is an actual stream. Closes ruby-grape#2087
1 parent ceca8d4 commit 907fbee

File tree

4 files changed

+17
-2
lines changed

4 files changed

+17
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
#### Fixes
88

9-
* Your contribution here.
9+
* [#2083](https://github.com/ruby-grape/grape/pull/2083): Set Cache-Control only for streamed responses - [@stanhu](https://github.com/stanhu).
1010

1111
### 1.4.0 (2020/07/10)
1212

lib/grape/dsl/inside_route.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,8 @@ def sendfile(value = nil)
328328
# * https://github.com/rack/rack/blob/99293fa13d86cd48021630fcc4bd5acc9de5bdc3/lib/rack/chunked.rb
329329
# * https://github.com/rack/rack/blob/99293fa13d86cd48021630fcc4bd5acc9de5bdc3/lib/rack/etag.rb
330330
def stream(value = nil)
331+
return if value.nil? && @stream.nil?
332+
331333
header 'Content-Length', nil
332334
header 'Transfer-Encoding', nil
333335
header 'Cache-Control', 'no-cache' # Skips ETag generation (reading the response up front)
@@ -339,7 +341,7 @@ def stream(value = nil)
339341
elsif !value.is_a?(NilClass)
340342
raise ArgumentError, 'Stream object must respond to :each.'
341343
else
342-
instance_variable_defined?(:@stream) ? @stream : nil
344+
@stream
343345
end
344346
end
345347

spec/grape/api_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,11 @@ class DummyFormatClass
11491149
expect(last_response.headers['Content-Type']).to eq('text/plain')
11501150
end
11511151

1152+
it 'does not set Cache-Control' do
1153+
get '/foo'
1154+
expect(last_response.headers['Cache-Control']).to eq(nil)
1155+
end
1156+
11521157
it 'sets content type for xml' do
11531158
get '/foo.xml'
11541159
expect(last_response.headers['Content-Type']).to eq('application/xml')

spec/grape/dsl/inside_route_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,13 @@ def initialize
351351
expect(subject.header['Cache-Control']).to eq 'no-cache'
352352
end
353353

354+
it 'does not change Cache-Control header' do
355+
subject.stream
356+
357+
expect(subject.header['Cache-Control']).to eq 'cache'
358+
end
359+
360+
354361
it 'sets Content-Length header to nil' do
355362
subject.stream file_path
356363

@@ -419,6 +426,7 @@ def initialize
419426

420427
it 'returns default' do
421428
expect(subject.stream).to be nil
429+
expect(subject.header['Cache-Control']).to eq nil
422430
end
423431
end
424432

0 commit comments

Comments
 (0)