swift-nio icon indicating copy to clipboard operation
swift-nio copied to clipboard

Support For Swift-HTTP-Types

Open MahdiBM opened this issue 1 year ago • 11 comments

What are the plans of this package to support swift-http-types? The best plan I can come up with is to use the Swift 6 package traits to have an option to switch to those types?

MahdiBM avatar Jun 27 '24 18:06 MahdiBM

I am currently using the conversion channel handlers in https://github.com/apple/swift-nio-extras/tree/main/Sources/NIOHTTPTypesHTTP1. But would love to see conversion straight to swift-http-types instead of going via the types in NIOHTTP1, but that would require implementing the whole of NIOHTTP1 using the new types so upgrade handlers etc are available.

adam-fowler avatar Jun 28 '24 07:06 adam-fowler

I would love to have swift-http-types supported. We're currently paying conversion costs

Joannis avatar Jun 28 '24 13:06 Joannis

I think at this point we should consider cutting over to the common types.

Lukasa avatar Jun 29 '24 13:06 Lukasa

@Lukasa So like in/with a new major version?

MahdiBM avatar Jun 29 '24 15:06 MahdiBM

I don’t think that’s necessary. We can deprecate the old APIs and keep them around.

Lukasa avatar Jun 29 '24 16:06 Lukasa

Ah ok that was kind of what I was thinking about as well, which made me think about "package traits". Though normal deprecation of the old APIs and addition of the new APIs could be enough.

MahdiBM avatar Jun 29 '24 16:06 MahdiBM

Bump - any thoughts on this? May i consider putting together a PR for this?

mpapple-swift avatar Oct 30 '24 21:10 mpapple-swift

This is more than just writing an encoder, decoder for the HTTP types. You also need to provide the other support channel handlers like pipelining, response header validation, error handling and client and server pipeline upgrades. Of course the encoder/decoder would be the starting point.

adam-fowler avatar Oct 31 '24 12:10 adam-fowler

Yeah, Adam is right that this is a bit of a changeover, but if you want to start with the encoder/decoder I think that's a good first step. I think we end up needing the following items changed (@adam-fowler please shout if I've missed anything):

  • New typealiases for the parts. Probably HTTPRequestPart and HTTPResponsePart, as they should now be symmetrical (we can take this opportunity to exercise the cursed IOData type)
  • New typealiases for the decoders.
  • New encoders.
  • Updates to HTTPDecoder to ensure it can handle the new part types
  • Update to HTTPDecoder to ensure it conforms to WriteObservingByteToMessageDecoder in the new contexts.
  • New HTTPServerPipelineHandler
  • New HTTPServerProtocolErrorHandler
  • New server upgrade handlers:
    • HTTPServerUpgradeHandler
    • HTTPServerProtocolUpgrader
    • HTTPServerUpgradeEvents
  • New client upgrade handlers:
    • NIOHTTPClientUpgradeHandler
    • NIOHTTPClientProtocolUpgrader
  • New NIOHTTPRequestHeadersValidator
  • New NIOHTTPResponseHeadersValidator
  • New NIOHTTPClientResponseAggregator (including new NIOHTTPClientResponseFull)
  • New NIOHTTPServerRequestAggregator (including new NIOHTTPServerRequestFull)
  • New typed upgrade handler:
    • NIOTypedHTTPClientProtocolUpgrader
    • NIOTypedHTTPClientUpgradeConfiguration
    • NIOTypedHTTPClientUpgradeHandler
    • NIOTypedHTTPServerProtocolUpgrader
    • NIOTypedHTTPServerUpgradeConfiguration
    • NIOTypedHTTPServerUpgradeHandler
    • NIOUpgradableHTTPClientPipelineConfiguration
    • NIOUpgradableHTTPServerPipelineConfiguration
  • New channel pipeline configurators (though we can get away with fewer, as many of these existed for backwards compatibility)
    • ChannelPipeline
      • addHTTPClientHandlers(position:leftOverBytesStrategy:enableOutboundHeaderValidation:encoderConfiguration:withClientUpgrade:)
      • configureHTTPServerPipeline(position:withPipeliningAssistance:withServerUpgrade:withErrorHandling:withOutboundHeaderValidation:withEncoderConfiguration:)
      • configureUpgradableHTTPServerPipeline(configuration:)
      • configureUpgradableHTTPClientPipeline(configuration:)
    • ChannelPipeline.SynchronousOperations
      • addHTTPClientHandlers(position:leftOverBytesStrategy:enableOutboundHeaderValidation:encoderConfiguration:withClientUpgrade:)
      • configureHTTPServerPipeline(position:withPipeliningAssistance:withServerUpgrade:withErrorHandling:withOutboundHeaderValidation:withEncoderConfiguration:)
      • configureUpgradableHTTPServerPipeline(configuration:)
      • configureUpgradableHTTPClientPipeline(configuration:)
  • Deprecate everything we no longer need, including all types replaced above and most of our other duplicative types.
  • All the existing tests duplicated to cover the new types

This is, I hope it's fairly easy to see, a fairly substantial chunk of work. While we do this I think the various types should remain internal to avoid exposing a partial new API, and to allow us to fix any mistakes in the flow. But we will happily accept PRs from folks who want to start cutting things over.

Lukasa avatar Oct 31 '24 14:10 Lukasa

@Lukasa one thing missed from this, Is what to do with the swift-nio-extras modules that we currently use to support the swift http types.

  • Would the plan be to add new swift http type code in NIOHTTP1? Otherwise need to be careful with module naming to not clash with swift-nio-extra modules.
  • Should HTTP2 conversion handler be moved into swift-nio-http2?
  • Also review what request validation is repeated in both decoder and in swift http type initialisation.

adam-fowler avatar Nov 06 '24 12:11 adam-fowler

Would the plan be to add new swift http type code in NIOHTTP1? Otherwise need to be careful with module naming to not clash with swift-nio-extra modules.

My preference would be yes, as we want to keep the access to the decoder and other types.

Should HTTP2 conversion handler be moved into swift-nio-http2?

Yes, but that work is on a different schedule.

Also review what request validation is repeated in both decoder and in swift http type initialisation.

Agreed.

Lukasa avatar Nov 06 '24 12:11 Lukasa