Skip to content

Commit

Permalink
fix: support non-lower-case Content-Type header provided by app (#516)
Browse files Browse the repository at this point in the history
* fix: support non-lower-case Content-Type header provided by app

The changes in #511 change what headers get set by PDFKit
and make all headers set by PDFKit lower-case.

However, the changes also affect code depending on headers
set by the app - which for Rack 2.x apps will be 'Content-Type'.

To address this, the code should check if Content-Type is present
and in that case use it, otherwise default to content-type.

As the code also sets this header (changing the content type
to 'application/pdf'), it should set the same header that
the original value is retrieved from.

So decide on the exact header name first, store it in the
'content_type_header' variable, and use it to index the
headers dict.

Fix #515.

* rspec: add tests for mixed case Content-Type header support

As rack 3.x outright rejects mixed case headers, this test has to be marked
as pending - but passes (without the pending flag) with rack 2.x.

* rspec/content-type: make mixed case test pending only on Rack>=3.0.0
  • Loading branch information
vladimir-mencl-eresearch committed Oct 12, 2022
1 parent 6213317 commit 089e5de
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/pdfkit/middleware.rb
Expand Up @@ -22,7 +22,8 @@ def _call(env)
status, headers, response = @app.call(env)

begin
if rendering_pdf? && headers['content-type'] =~ /text\/html|application\/xhtml\+xml/
content_type_header = headers.has_key?('Content-Type') ? 'Content-Type' : 'content-type'
if rendering_pdf? && headers[content_type_header] =~ /text\/html|application\/xhtml\+xml/
body = response.respond_to?(:body) ? response.body : response.join
body = body.join if body.is_a?(Array)

Expand All @@ -49,7 +50,7 @@ def _call(env)
end

headers['content-length'] = (body.respond_to?(:bytesize) ? body.bytesize : body.size).to_s
headers['content-type'] = 'application/pdf'
headers[content_type_header] = 'application/pdf'
headers['content-disposition'] ||= @conditions[:disposition] || 'inline'
end
rescue StandardError => e
Expand Down
23 changes: 23 additions & 0 deletions spec/middleware_spec.rb
Expand Up @@ -422,6 +422,29 @@ def mock_app(options = {}, conditions = {}, custom_headers = {})
end
end

describe "content type header" do
before { mock_app }

context "lower case" do
specify "header gets correctly updated" do
get 'http://www.example.org/public/test.pdf'
expect(last_response.headers["content-type"]).to eq("application/pdf")
end
end

context "mixed case" do
let(:headers) do
{'Content-Type' => "text/html"}
end

specify "header gets correctly updated" do
pending("this test only applies to rack 2.x and is rejected by rack 3.x") if Rack.release >= "3.0.0"
get 'http://www.example.org/public/test.pdf'
expect(last_response.headers["Content-Type"]).to eq("application/pdf")
end
end
end

describe "remove .pdf from PATH_INFO and REQUEST_URI" do
before { mock_app }

Expand Down

0 comments on commit 089e5de

Please sign in to comment.