webmachine-ruby icon indicating copy to clipboard operation
webmachine-ruby copied to clipboard

404 with encodings_provided sets encoding Header but doesn't encode response body

Open Asmod4n opened this issue 11 years ago • 7 comments

#!/usr/bin/env ruby

require 'bundler/setup'
require 'webmachine'

class Resource < Webmachine::Resource
  def resource_exists?
    false
  end

  def encodings_provided
    { 'gzip' => :encode_gzip,
      'deflate' => :encode_deflate,
      'identity' => :encode_identity }
  end

  def to_html
    "Hello"
  end
end

Webmachine.application.routes do
  add_route ['*'], Resource
end

Webmachine.run

This is the reply

HTTP/1.1 404 Not Found 
Content-Type: text/html
Content-Encoding: gzip
Vary: Accept-Encoding
Content-Length: 213
Date: Sat, 04 Jan 2014 22:48:04 GMT
Server: Webmachine-Ruby/1.2.1 WEBrick/1.3.1 (Ruby/2.1.0/2013-12-25)
Connection: Keep-Alive

<!DOCTYPE html><html> <head><title>404 Not Found</title></head> <body><h1>404 Not Found</h1><p>The requested document was not found on this server.</p> <address>Webmachine-Ruby/1.2.1 server</address></body></html>

Asmod4n avatar Jan 04 '14 22:01 Asmod4n

the problem is here: https://github.com/seancribbs/webmachine-ruby/blob/master/lib/webmachine/decision/fsm.rb#L61

ghost avatar Jan 04 '14 22:01 ghost

A dirty trick to resolve it would be to unset the Content-Encoding header in https://github.com/seancribbs/webmachine-ruby/blob/master/lib/webmachine/errors.rb#L14 because the length is also for the unencoded response.

Asmod4n avatar Jan 04 '14 23:01 Asmod4n

unset the Content-Encoding header

That sounds like the simplest thing to do. We'll know once someone implements it :)

ghost avatar Jan 20 '14 03:01 ghost

Close and mark as wont-fix for now until someone else wants it?

bethesque avatar Jan 15 '15 06:01 bethesque

Dunno if I would close it, since it is a bug

Asmod4n avatar Jan 15 '15 06:01 Asmod4n

Tis. But you don't have to fix every bug ;) That's why the 'wont-fix' tag exists!

It seems like a low impact bug which nobody is enthusiastic about fixing, so I'm not sure if there's much point in leaving it open. But we can leave it if nobody else shares my particular obsession with checking items off lists!

bethesque avatar Jan 15 '15 07:01 bethesque

I wanted to understand this issue better so I first created a resource with a to_html method containing the current 404 not found response body.

class Resource < Webmachine::Resource
  def resource_exists?
    true
  end

  def to_html
    "<!DOCTYPE html><html>\n <head><title>404 Not Found</title></head>\n <body><h1>404 Not Found</h1>\n <p>The requested document was not found on this server.</p>\n <address>Webmachine-Ruby/1.4.0 server</address></body></html>\n"
  end
end

The response: Content-Length: 219

Then I added the encodings_provided method.

class Resource < Webmachine::Resource
  def resource_exists?
    false
  end

  def encodings_provided
    { 'gzip' => :encode_gzip,
      'deflate' => :encode_deflate,
      'identity' => :encode_identity }
  end

  def to_html
    "<!DOCTYPE html><html>\n <head><title>404 Not Found</title></head>\n <body><h1>404 Not Found</h1>\n <p>The requested document was not found on this server.</p>\n <address>Webmachine-Ruby/1.4.0 server</address></body></html>\n"
  end
end

The response: Content-Length: 175 Content-Encoding: gzip

Here it works because it was a 200 response, not a 404 response. The path it took through the FSM was different.

Now I set the resource_exists? to false with no encodings_provided method present.

class Resource < Webmachine::Resource
  def resource_exists?
    false
  end

  def to_html
    "hello"
  end
end

The response: Content-Length: 219

And lastly I added the encodings_provided method.

class Resource < Webmachine::Resource
  def resource_exists?
    false
  end

  def encodings_provided
    { 'gzip' => :encode_gzip,
      'deflate' => :encode_deflate,
      'identity' => :encode_identity }
  end

  def to_html
    "hello"
  end
end

The response: Content-Length: 219 Content-Encoding: gzip

The problem is that Content-Length should have been 175.

render_error creates a response body when no body is given. A response body can be encoded when you know which encodings are provided by the resource. So I think this can't happen inside render_error because the resource isn't available there.

A 404 response can be given because:

  1. The Dispatcher can't find a resource for a request. In this case there is no custom response body possible but also encodings_provided isn't available at all because the resource doesn't exist.
  2. resource_exists? has been set to false. Here a custom response body should be possible and encodings_provided can be used.

I think to fix the issue for 2. the l7 state in the FSM should set up the response body and encode it before the response is passed to the render_error method. (The state diagram for reference.)

(I tried to fix it by unsetting the Content-Encoding header in render_error but this didn't had an effect.)

jimvm avatar Oct 11 '15 12:10 jimvm