From 723b5384f2dcd6541477a42262f75e45276b595c Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Thu, 19 Jan 2023 16:56:56 -0800 Subject: [PATCH] Add general `Rack::BadRequest`. (#2019) Used to communicate a class of exceptions that represent 400 Bad Request semantics. --- CHANGELOG.md | 4 ++++ lib/rack.rb | 1 + lib/rack/bad_request.rb | 6 ++++++ lib/rack/multipart.rb | 3 +++ lib/rack/multipart/parser.rb | 18 ++++++++++++++---- lib/rack/query_parser.rb | 14 +++++++++++--- test/spec_multipart.rb | 2 +- 7 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 lib/rack/bad_request.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fcc45a79..c852a84a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes to this project will be documented in this file. For info on - `rack.input` is now optional. ([#1997](https://github.com/rack/rack/pull/1997), [@ioquatix]) +### Changed + +- Improved handling of multipart requests. `rack.input` is now optional, and if missing, will raise an error which includes `module Rack::BadRequest`. Several other exceptions also include this module. ([#1997](https://github.com/rack/rack/pull/1997), [@ioquatix]) + ## [3.0.3] - 2022-12-07 ### Fixed diff --git a/lib/rack.rb b/lib/rack.rb index 5b87ea1bc..2dd8d3dc2 100644 --- a/lib/rack.rb +++ b/lib/rack.rb @@ -15,6 +15,7 @@ require_relative 'rack/constants' module Rack + autoload :BadRequest, "rack/bad_request" autoload :Builder, "rack/builder" autoload :BodyProxy, "rack/body_proxy" autoload :Cascade, "rack/cascade" diff --git a/lib/rack/bad_request.rb b/lib/rack/bad_request.rb new file mode 100644 index 000000000..9c6751260 --- /dev/null +++ b/lib/rack/bad_request.rb @@ -0,0 +1,6 @@ +module Rack + # Represents a 400 Bad Request error when input data fails to meet the + # requirements. + module BadRequest + end +end diff --git a/lib/rack/multipart.rb b/lib/rack/multipart.rb index 76b25a51b..165b4db3a 100644 --- a/lib/rack/multipart.rb +++ b/lib/rack/multipart.rb @@ -6,6 +6,8 @@ require_relative 'multipart/parser' require_relative 'multipart/generator' +require_relative 'bad_request' + module Rack # A multipart form data parser, adapted from IOWA. # @@ -14,6 +16,7 @@ module Multipart MULTIPART_BOUNDARY = "AaB03x" class MissingInputError < StandardError + include BadRequest end class << self diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index ceafbc6b7..c49c80e6a 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -3,18 +3,28 @@ require 'strscan' require_relative '../utils' +require_relative '../bad_request' module Rack module Multipart - class MultipartPartLimitError < Errno::EMFILE; end + class MultipartPartLimitError < Errno::EMFILE + include BadRequest + end # Use specific error class when parsing multipart request # that ends early. - class EmptyContentError < ::EOFError; end + class EmptyContentError < ::EOFError + include BadRequest + end # Base class for multipart exceptions that do not subclass from # other exception classes for backwards compatibility. - class Error < StandardError; end + class BoundaryTooLongError < StandardError + include BadRequest + end + + # Prefer to use the BoundaryTooLongError class or Rack::BadRequest. + Error = BoundaryTooLongError EOL = "\r\n" MULTIPART = %r|\Amultipart/.*boundary=\"?([^\";,]+)\"?|ni @@ -96,7 +106,7 @@ def self.parse(io, content_length, content_type, tmpfile, bufsize, qp) if boundary.length > 70 # RFC 1521 Section 7.2.1 imposes a 70 character maximum for the boundary. # Most clients use no more than 55 characters. - raise Error, "multipart boundary size too large (#{boundary.length} characters)" + raise BoundaryTooLongError, "multipart boundary size too large (#{boundary.length} characters)" end io = BoundedIO.new(io, content_length) if content_length diff --git a/lib/rack/query_parser.rb b/lib/rack/query_parser.rb index a89c4c7f4..7c9621ed5 100644 --- a/lib/rack/query_parser.rb +++ b/lib/rack/query_parser.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative 'bad_request' + module Rack class QueryParser DEFAULT_SEP = /[&] */n @@ -7,16 +9,22 @@ class QueryParser # ParameterTypeError is the error that is raised when incoming structural # parameters (parsed by parse_nested_query) contain conflicting types. - class ParameterTypeError < TypeError; end + class ParameterTypeError < TypeError + include BadRequest + end # InvalidParameterError is the error that is raised when incoming structural # parameters (parsed by parse_nested_query) contain invalid format or byte # sequence. - class InvalidParameterError < ArgumentError; end + class InvalidParameterError < ArgumentError + include BadRequest + end # ParamsTooDeepError is the error that is raised when params are recursively # nested over the specified limit. - class ParamsTooDeepError < RangeError; end + class ParamsTooDeepError < RangeError + include BadRequest + end def self.make_default(param_depth_limit) new Params, param_depth_limit diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index 7097528ae..3149dd6b2 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -38,7 +38,7 @@ def multipart_file(name) env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_filename, "A"*71)) lambda { Rack::Multipart.parse_multipart(env) - }.must_raise Rack::Multipart::Error + }.must_raise Rack::Multipart::BoundaryTooLongError end it "raises a bad request exception if no body is given but content type indicates a multipart body" do