Bug: deserializing malformed basic auth token results in java.lang.IllegalArgumentException raised outside of F

[grouzen]

The exception is being raised when calling BasicCredentials.unapply method in pattern matching statement at https://github.com/http4s/http4s/blob/series/0.23/server/shared/src/main/scala/org/http4s/server/middleware/authentication/BasicAuth.scala#L69

It causes the situation when you need to catch an exception using try/catch instead of leveraging cats-effect machinery.

java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 132
	at java.base/java.util.Base64$Decoder.decode0(Base64.java:774)
	at java.base/java.util.Base64$Decoder.decode(Base64.java:538)
	at java.base/java.util.Base64$Decoder.decode(Base64.java:561)
	at org.http4s.BasicCredentials$.apply(Credentials.scala:106)
	at org.http4s.BasicCredentials$.unapply(Credentials.scala:118)
	at org.http4s.server.middleware.authentication.BasicAuth$.validatePassword(BasicAuth.scala:69)
	at org.http4s.server.middleware.authentication.BasicAuth$.$anonfun$challenge$1(BasicAuth.scala:57)

I think it should be considered a bug.

[armanbilge]

Thanks. I guess the problem is here, this method is definitely not total if it's expecting a Base64 string.

http4s/core/shared/src/main/scala/org/http4s/Credentials.scala

Lines 105 to 106 in 0c38b06

	def apply(token: String): BasicCredentials = {
	val bytes = Base64.getDecoder.decode(token)

[grouzen]

Yes, indeed. I would like to fix it. Do you have any idea of how to do this properly?
I believe we can't change the return type of the existing apply method as it leads to binary incompatibility.

[armanbilge]

Awesome, thanks! I guess we can deprecate the apply method and replace it with a fromString that either turns Either or Option. I think we use a similar API style in other places.

[grouzen]

Great! I'll fix it then.