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

Models the Max-Forwards header #3577

Merged
merged 1 commit into from Jul 12, 2020
Merged

Models the Max-Forwards header #3577

merged 1 commit into from Jul 12, 2020

Conversation

cquiroz
Copy link
Member

@cquiroz cquiroz commented Jul 12, 2020

Simple modeling of Max-Forwards as part of #2011

Signed-off-by: Carlos Quiroz <carlos.m.quiroz@gmail.com>
import org.http4s.util.Writer

// see https://tools.ietf.org/html/rfc7231#section-5.1.2
sealed abstract case class `Max-Forwards`(count: Long) extends Header.Parsed {
Copy link
Member

@rossabaker rossabaker Jul 12, 2020

Choose a reason for hiding this comment

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

Pedantically, there's no limit, and Long.MaxValue + 1 would be a legal value. It's also absurd. I think Long is the pragmatic model here, even if it's strictly incorrect.

Copy link
Member Author

@cquiroz cquiroz Jul 12, 2020

Choose a reason for hiding this comment

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

That's correct, i followed the same logic as for Content-length, in practice an Int probably would suffice

Copy link
Member

@rossabaker rossabaker Jul 12, 2020

Choose a reason for hiding this comment

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

It's possible do do this (and Content-Length) the pedantic way. Uncompiled:

sealed abstract class `Max-Forwards` {
  def unsafeToLong: Long
  def toLong: Option[Long]
  def toBigInt: BigInt
}

object `Max-Forwards` {
  private final class AtLong(val unsafeToLong: Long) extends `Max-Forwards` {
     val toLong = Some(unsafeToLong)
     val toBigInt = unsafeToLong.toBigInt
  }

  private final class AtBigInt(val toBigInt: BigInt) extends `Max-Forwards` {
    def toLong = if (toBigInt > BigInt.valueOf(Long.MaxValue)) None else Some(unsafeToLong)
    def unsafeToLong = toBigInt.longValue
  }
}

What I don't like about it is most people will just want a Long, and we're shaming that method as unsafe. We could just return the last 64 bits, which is what 99.999999999999% of messages want, and what the standard library does.

Or we could continue to just assume nobody does this in practice. I'm just thinking out loud.

@cquiroz cquiroz merged commit 6ae3b30 into http4s:master Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants