Fix Grape allowing invalid headers to be set #2511
Conversation
Here's the expected TOC for README.md: # Table of Contents
- [What is Grape?](#what-is-grape)
- [Stable Release](#stable-release)
- [Project Resources](#project-resources)
- [Grape for Enterprise](#grape-for-enterprise)
- [Installation](#installation)
- [Basic Usage](#basic-usage)
- [Rails 7.1](#rails-71)
- [Mounting](#mounting)
- [All](#all)
- [Rack](#rack)
- [Alongside Sinatra (or other frameworks)](#alongside-sinatra-or-other-frameworks)
- [Rails](#rails)
- [Zeitwerk](#zeitwerk)
- [Modules](#modules)
- [Remounting](#remounting)
- [Mount Configuration](#mount-configuration)
- [Versioning](#versioning)
- [Strategies](#strategies)
- [Path](#path)
- [Header](#header)
- [Accept-Version Header](#accept-version-header)
- [Param](#param)
- [Describing Methods](#describing-methods)
- [Configuration](#configuration)
- [Parameters](#parameters)
- [Params Class](#params-class)
- [Declared](#declared)
- [Include Parent Namespaces](#include-parent-namespaces)
- [Include Missing](#include-missing)
- [Evaluate Given](#evaluate-given)
- [Parameter Precedence](#parameter-precedence)
- [Parameter Validation and Coercion](#parameter-validation-and-coercion)
- [Supported Parameter Types](#supported-parameter-types)
- [Integer/Fixnum and Coercions](#integerfixnum-and-coercions)
- [Custom Types and Coercions](#custom-types-and-coercions)
- [Multipart File Parameters](#multipart-file-parameters)
- [First-Class JSON Types](#first-class-json-types)
- [Multiple Allowed Types](#multiple-allowed-types)
- [Validation of Nested Parameters](#validation-of-nested-parameters)
- [Dependent Parameters](#dependent-parameters)
- [Group Options](#group-options)
- [Renaming](#renaming)
- [Built-in Validators](#built-in-validators)
- [allow_blank](#allow_blank)
- [values](#values)
- [except_values](#except_values)
- [same_as](#same_as)
- [length](#length)
- [regexp](#regexp)
- [mutually_exclusive](#mutually_exclusive)
- [exactly_one_of](#exactly_one_of)
- [at_least_one_of](#at_least_one_of)
- [all_or_none_of](#all_or_none_of)
- [Nested mutually_exclusive, exactly_one_of, at_least_one_of, all_or_none_of](#nested-mutually_exclusive-exactly_one_of-at_least_one_of-all_or_none_of)
- [Namespace Validation and Coercion](#namespace-validation-and-coercion)
- [Custom Validators](#custom-validators)
- [Validation Errors](#validation-errors)
- [I18n](#i18n)
- [Custom Validation messages](#custom-validation-messages)
- [presence, allow_blank, values, regexp](#presence-allow_blank-values-regexp)
- [same_as](#same_as-1)
- [length](#length-1)
- [all_or_none_of](#all_or_none_of-1)
- [mutually_exclusive](#mutually_exclusive-1)
- [exactly_one_of](#exactly_one_of-1)
- [at_least_one_of](#at_least_one_of-1)
- [Coerce](#coerce)
- [With Lambdas](#with-lambdas)
- [Pass symbols for i18n translations](#pass-symbols-for-i18n-translations)
- [Overriding Attribute Names](#overriding-attribute-names)
- [With Default](#with-default)
- [Using dry-validation or dry-schema](#using-dry-validation-or-dry-schema)
- [Headers](#headers)
- [Request](#request)
- [Header Value Types](#header-value-types)
- [Header Case Handling](#header-case-handling)
- [Response](#response)
- [Routes](#routes)
- [Helpers](#helpers)
- [Path Helpers](#path-helpers)
- [Parameter Documentation](#parameter-documentation)
- [Cookies](#cookies)
- [HTTP Status Code](#http-status-code)
- [Redirecting](#redirecting)
- [Recognizing Path](#recognizing-path)
- [Allowed Methods](#allowed-methods)
- [Raising Exceptions](#raising-exceptions)
- [Default Error HTTP Status Code](#default-error-http-status-code)
- [Handling 404](#handling-404)
- [Exception Handling](#exception-handling)
- [Rescuing exceptions inside namespaces](#rescuing-exceptions-inside-namespaces)
- [Unrescuable Exceptions](#unrescuable-exceptions)
- [Exceptions that should be rescued explicitly](#exceptions-that-should-be-rescued-explicitly)
- [Logging](#logging)
- [API Formats](#api-formats)
- [JSONP](#jsonp)
- [CORS](#cors)
- [Content-type](#content-type)
- [API Data Formats](#api-data-formats)
- [JSON and XML Processors](#json-and-xml-processors)
- [RESTful Model Representations](#restful-model-representations)
- [Grape Entities](#grape-entities)
- [Hypermedia and Roar](#hypermedia-and-roar)
- [Rabl](#rabl)
- [Active Model Serializers](#active-model-serializers)
- [Sending Raw or No Data](#sending-raw-or-no-data)
- [Authentication](#authentication)
- [Basic Auth](#basic-auth)
- [Register custom middleware for authentication](#register-custom-middleware-for-authentication)
- [Describing and Inspecting an API](#describing-and-inspecting-an-api)
- [Current Route and Endpoint](#current-route-and-endpoint)
- [Before, After and Finally](#before-after-and-finally)
- [Anchoring](#anchoring)
- [Instance Variables](#instance-variables)
- [Using Custom Middleware](#using-custom-middleware)
- [Grape Middleware](#grape-middleware)
- [Rails Middleware](#rails-middleware)
- [Remote IP](#remote-ip)
- [Writing Tests](#writing-tests)
- [Writing Tests with Rack](#writing-tests-with-rack)
- [RSpec](#rspec)
- [Airborne](#airborne)
- [MiniTest](#minitest)
- [Writing Tests with Rails](#writing-tests-with-rails)
- [RSpec](#rspec-1)
- [MiniTest](#minitest-1)
- [Stubbing Helpers](#stubbing-helpers)
- [Reloading API Changes in Development](#reloading-api-changes-in-development)
- [Reloading in Rack Applications](#reloading-in-rack-applications)
- [Reloading in Rails Applications](#reloading-in-rails-applications)
- [Performance Monitoring](#performance-monitoring)
- [Active Support Instrumentation](#active-support-instrumentation)
- [endpoint_run.grape](#endpoint_rungrape)
- [endpoint_render.grape](#endpoint_rendergrape)
- [endpoint_run_filters.grape](#endpoint_run_filtersgrape)
- [endpoint_run_validators.grape](#endpoint_run_validatorsgrape)
- [format_response.grape](#format_responsegrape)
- [Monitoring Products](#monitoring-products)
- [Contributing to Grape](#contributing-to-grape)
- [Security](#security)
- [License](#license)
- [Copyright](#copyright)Generated by 🚫 Danger |
There was a problem hiding this comment.
Thanks.
This needs some docs (README, CHANGELOG, possibly UPGRADING depending on the answers to below).
- What is the current behavior of
header 'integer key', 123? I am afraid we may be breaking some existing behavior. - I don't think we need a warning if we're going to automatically coerce every header value
.to_s, we should probably either loudly fail or document it as such.
Fixes ruby-grape#2334 Ensure all header values are strings according to the Rack spec. * Convert header values to strings using `to_s` in the `header` method in `lib/grape/dsl/headers.rb`. * Emit a warning if the header value is not a string in the `header` method in `lib/grape/dsl/headers.rb`. * Add tests in `spec/grape/dsl/headers_spec.rb` to verify that non-string header values are converted to strings and warnings are emitted. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/ruby-grape/grape/issues/2334?shareId=XXXX-XXXX-XXXX-XXXX).
SlakrHakr
force-pushed
the
convert-headers-to-strings
branch
from
80cacb7
to
6ffceb5
Compare
fixed an existing test that shows the difference with non-string headers before and after this change |
|
added documentation to README, CHANGELOG based on my answer above, is documentation in UPGRADING needed? |
The fact that a warning is emitted now will surprise users, and we are trying them to change the code to do an explicit @SlakrHakr the TOC needs a fix, see danger complaining. @ericproulx WDYT about this change? |
|
fixed TOC and added documentation to UPGRADING doc |
It makes sense but maybe we should emit a Deprecation instead of a warn. RSpec can be configured with |
|
This suggestion could be a nice feature since its hard to comply 100% to the rack-spec. I think we could add something like |
|
I think I like a deprecation more as well. WDYT @SlakrHakr ? |
|
@SlakrHakr do you need some help ? I might make a release this weekend and your PR could be a part of it. Thanks :) |
Fixes #2334
Ensure all header values are strings according to the Rack spec.
to_sin theheadermethod inlib/grape/dsl/headers.rb.headermethod inlib/grape/dsl/headers.rb.spec/grape/dsl/headers_spec.rbto verify that non-string header values are converted to strings and warnings are emitted.For more details, open the Copilot Workspace session.