search

pimeys / rust-web-push

A Web Push library for Rust
Apache License 2.0
114 stars 21 forks source link

Fixed issue with implicit ttl being not valid #6

Closed JCapucho closed 5 years ago

JCapucho commented 5 years ago

The new revisions of web push state that the ttl header must be set explicitly or an bad request error will be returned and the message will not be delivered, so why propose that we set the ttl header automatically to the highest number possible.

https://blog.mozilla.org/services/2016/02/20/webpushs-new-requirement-ttl-header/

pimeys commented 5 years ago

Hey, thanks for the PR. Here reading my notifications in the forest, so a bit of lag :)

Would it be better to allow configuration of the value, the default being the given big number?

Also would make it a bit easier for me to read the PR without the automatic rustfmt. This crate has been written before rustfmt was such a thing, so I understand the eagerness for the formatter...

JCapucho commented 5 years ago

Thanks for responding-

Would it be better to allow configuration of the value, the default being the given big number?

In the proposed change the default number when new() is called is the biggest possible but it can be changed by calling set_ttl(u32)

I'm sorry for the rustfmt but i'm using vs code and i've set to format on save

pimeys commented 5 years ago

Yeah, I have the same setting nowadays in my Emacs.

Sorry for the late response, been having a vacation so not really reading my notifications :).

I'm now getting this to compile in my openbsd laptop and trying to see your changes at the same time. Maybe even making a new version release!

JCapucho commented 5 years ago

That's great. But I think we should remove the option as it can longer be implicit, when I changed I left because I was afraid of breaking anything and ad spent 2 days figuring out what was going on.

pimeys commented 5 years ago

Btw, I'm not working in this space anymore, so how is the library working for your purposes? I vaguely remember writing the VAPID part before we went bankrupt, but was never able to test it with real traffic. If I remember right there was some weird behaviour when VAPID worked on Firefox but never with Chrome. Have you had better luck with it?

Yeah, let's not break it for others. I had also very hard time writing this and reading all the different web push and http-ece RFC's trying to get all of it to work. The whole standard is such a mess to implement :(

JCapucho commented 5 years ago

I've a very basic implementation of the library in my application but it's working right even in Firefox.

pimeys commented 5 years ago

Released 0.4.2

https://crates.io/crates/web-push