[satsharm]

Why this PR is needed?
sni function is supposed to provide the value from request and if we pass mut ref as parameter then it is bound to change the value of sni which is wrong so I removed mut ref from sni function parameter.

[hcldan]

I believe this is also preventing us from being able to use it in a handler where the reference to the request is not mutable.

[hcldan]

@the10thWiz I believe you did a lot of this work to get the feature into Rocket (thank you, btw). Could you please review this PR?

[tarpboi]

I don't think this PR is needed, ty anyway!

[martynp]

This exposes the sni in on_request fairings - which can be useful.

More importantly, I don't think this breaks anything.

[hcldan]

@martynp This doesn't expose anything new, it changes the existing exposure to be non-mutable so that we can access it when we do not have a mutable reference to the request.

I don't believe the intent was to allow this to be changeable after it is set by the rocket internals.

[hcldan]

I don't think this PR is needed, ty anyway!

@HarrierOnChain Can you please elaborate? We would not have offered this PR if we knew we were able to proceed without it.

[martynp]

@martynp This doesn't expose anything new, it changes the existing exposure to be non-mutable so that we can access it when we do not have a mutable reference to the request.

I don't believe the intent was to allow this to be changeable after it is set by the rocket internals.

This has been requested in the past, there is an open PR with it somewhere.

As on_request cannot modify the request it is not currently available to on_request fairings.

Knowing the sni in an on_request fairing is useful.

As this change would make something available which was previously not available I used the term 'expose'. I am not sure I am incorrect in that regard.

[hcldan]

@martynp This doesn't expose anything new, it changes the existing exposure to be non-mutable so that we can access it when we do not have a mutable reference to the request.
I don't believe the intent was to allow this to be changeable after it is set by the rocket internals.

This has been requested in the past, there is an open PR with it somewhere.

As on_request cannot modify the request it is not currently available to on_request fairings.

Knowing the sni in an on_request fairing is useful.

As this change would make something available which was previously not available I used the term 'expose'. I am not sure I am incorrect in that regard.

Thanks for the clarification! Agreed.

[hcldan]

@SergioBenitez as this is a tiny change, do you think it could be merged soon?

[SergioBenitez]

@hcldan In theory yes. I am working on a bunch of changes for a v0.6-rc release, which includes a change to the HTTP3 support, which touches this. As long as that doesn't conflict, I will merge this.

[hcldan]

@SergioBenitez Thank you for working on bringing that in to the v0.6-rc release. I don't think this will cause any conflicts.

Are you close to releasing the RC? We need an official release (rc or otherwise) to keep working on our project. Using the git as the source for our rocket dependency is causing problems with our build tools.

[SergioBenitez]

@hcldan I was waiting on upstream changes to be merged; that just happened now. Will need them to release before we can. Hopefully this weekend, however.

[hcldan]

@SergioBenitez please let us know if there is anything we can do to help.

[SergioBenitez]

@hcldan I've unfortunately landed in the hospital, so this will have to wait until I'm out. At least a week or two.

[hcldan]

@SergioBenitez Wishing you a speedy recovery!