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.

Sign up for GitHub

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

Jump to bottom

Replace Accept-Ranges parser with cats-parse implementation #3995

Merged
merged 3 commits into from Dec 10, 2020
Merged

Replace Accept-Ranges parser with cats-parse implementation #3995

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de6c3df
Replace `Accept-Ranges` parser with cats-parse implementation
fredshonorio Dec 8, 2020
bdceb50
Make `Accept-Ranges` parser match spec, clean up parboiled2 version
fredshonorio Dec 9, 2020
5f0072e
Update Accept-Ranges.scala
rossabaker Dec 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 30 additions & 3 deletions core/src/main/scala/org/http4s/headers/Accept-Ranges.scala
View file
Open in desktop
Expand Up @@ -6,8 +6,9 @@

package org.http4s
package headers

import org.http4s.parser.HttpHeaderParser
import cats.parse.{Parser => P}
import cats.implicits._
import org.http4s.internal.parsing.Rfc7230
import org.http4s.util.Writer

object `Accept-Ranges` extends HeaderKey.Internal[`Accept-Ranges`] with HeaderKey.Singleton {
Expand All @@ -16,7 +17,33 @@ object `Accept-Ranges` extends HeaderKey.Internal[`Accept-Ranges`] with HeaderKe
def none: `Accept-Ranges` = apply(Nil)

override def parse(s: String): ParseResult[`Accept-Ranges`] =
HttpHeaderParser.ACCEPT_RANGES(s)
parser.parseAll(s).leftMap { e =>
ParseFailure("Invalid accept-ranges", e.toString)
}

/* https://tools.ietf.org/html/rfc7233#appendix-C */
val parser: P[`Accept-Ranges`] = {
val ListSep: P[Unit] = Rfc7230.ows *> P.char(',') *> Rfc7230.ows
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will do a fine job on the ows-comma-ows that reasonable actors will send, but not the empty elements between commas (including leading and trailing commas) that the spec mandates. Rfc7230.headerRep1 should take care of those.


val none = P.string1("none").as(Nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good use of as.


val rangeUnit = Rfc7230.token.map(org.http4s.RangeUnit.apply)

/*
Accept-Ranges = acceptable-ranges
OWS = <OWS, see [RFC7230], Section 3.2.3>
acceptable-ranges = ( *( "," OWS ) range-unit *( OWS "," [ OWS range-unit ] ) ) / "none"
*/
val acceptableRanges: P[List[RangeUnit]] =
P.oneOf(
List(
none,
P.repSep(rangeUnit, 1, ListSep)
)
)

acceptableRanges.map(headers.`Accept-Ranges`.apply) /* is EOL necessary? */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is correct. Calling parseAll means Accept-Ranges.parse will have to consume the entire input, and if this is composed into other parsers, we want it to leave the leftovers.

}
}

final case class `Accept-Ranges` private[http4s] (rangeUnits: List[RangeUnit])
Expand Down
2 changes: 1 addition & 1 deletion tests/src/test/scala/org/http4s/parser/AcceptRangesSpec.scala
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ import org.http4s.headers.`Accept-Ranges`
import org.specs2.mutable.Specification

class AcceptRangesSpec extends Specification with HeaderParserHelper[`Accept-Ranges`] {
def hparse(value: String) = HttpHeaderParser.ACCEPT_RANGES(value)
def hparse(value: String) = `Accept-Ranges`.parse(value)

"Accept-Ranges header" should {
val ranges = List(
Expand Down