Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multipart forms without explicit Conent-Length header #2150

Closed
gsamokovarov opened this issue Dec 20, 2018 · 0 comments

Comments

Projects
None yet
3 participants
@gsamokovarov
Copy link

commented Dec 20, 2018

Uploading multipart forms without setting the Content-Length header fails for Passenger on Rack 2.x releases. This is because Rack itself does not conform to it's Input Stream SPEC and calls an #eof? on an input stream in the case of no Content-Length header:

      def self.parse(io, content_length, content_type, tmpfile, bufsize, qp)
        return EMPTY if 0 == content_length

        boundary = parse_boundary content_type
        return EMPTY unless boundary

        io = BoundedIO.new(io, content_length) if content_length

        parser = new(boundary, tmpfile, bufsize, qp)
        parser.on_read io.read(bufsize), io.eof?

        loop do
          break if parser.state == :DONE
          parser.on_read io.read(bufsize), io.eof?
        end

        io.rewind
        parser.result
      end

https://github.com/rack/rack/blob/8376dd11e6526a53432ee59b7a5d092bda9fc901/lib/rack/multipart/parser.rb#L59-L77

At this point, the io is of type PhusionPassenger::Utils::TeeInput, which does not implement the #eof?method. This is pretty legit, as the Rack SPEC does not require it. However, in the case of no content length, no BoundedIO wrapping is happening, which leaves a raw call to the #eof? method of PhusionPassenger::Utils::TeeInput, which leads to:

NoMethodError: undefined method `eof?' for #<PhusionPassenger::Utils::TeeInput:0x0000563c49844cf8>

This is where it gets interesting. This problem was found out and fixed in 2017 by rack/rack#1201. The commit sat in master and wasn't ported to rack's 2-0-stable branch, where the 2.* are built from. Yesterday the rack maintainers backported the change into the 2-0-stable branch, but no release have been made yet.

While you are completely complying to the spec, it may be a good idea to put the #eof? method on the TeeInput class for the folks who cannot run on 2-0-stable or are locked to a specific rack version, but can fix the issue by updating Passenger.

CamJN added a commit that referenced this issue Feb 4, 2019

Made Passenger more resiliant to Rack bugs. Closes GH-2150.
Rack keeps trying to call eof? on objects that don’t have that method. So implement it for the sake of not having to deal with this anymore.

CamJN added a commit that referenced this issue Feb 14, 2019

Made Passenger more resiliant to Rack bugs. Closes GH-2150.
Rack keeps trying to call eof? on objects that don’t have that method. So implement it for the sake of not having to deal with this anymore.

@CamJN CamJN closed this Feb 15, 2019

CamJN added a commit that referenced this issue Feb 15, 2019

Made Passenger more resiliant to Rack bugs. Closes GH-2150.
Rack keeps trying to call eof? on objects that don’t have that method. So implement it for the sake of not having to deal with this anymore.

@CamJN CamJN removed the SupportCentral label Feb 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.