ssrf_filter icon indicating copy to clipboard operation
ssrf_filter copied to clipboard

Breaking change introduced in 1.1.0

Open mshibuya opened this issue 2 years ago • 2 comments

The pull request #51 changed the usage of the block on calling SsrfFilter methods. Previously it received a request object as an argument but now it receives a response object. https://github.com/arkadiyt/ssrf_filter/pull/51/files#diff-95bca3ab492cb05975c93fdbbd14a18288350481b2f06307f2699fdd3fdb17d6L192-R184 Because of this, many CarrierWave users are facing issues. https://github.com/carrierwaveuploader/carrierwave/issues/2625

But this was a minor version bump. Since the project states SemVer compliance, it mustn't break backward compatibility of the public API. So my proposal is to treat the block given as a request_proc, and add a new argument response_proc which receives a response object as chunked-processing goes by. Another option will be making it a major version bump. If you consider that public API change crucial, this is the way to go.

I need @arkadiyt 's attention for this. Thanks.

mshibuya avatar Sep 10 '22 08:09 mshibuya

Sorry about this error y'all, I will address this tonight

arkadiyt avatar Sep 15 '22 19:09 arkadiyt

@arkadiyt Could you take a look at the PR #57 ? That's the fix I'm proposing, to keep the compatibility with pre-1.1.0.

mshibuya avatar Sep 16 '22 14:09 mshibuya

Hey folks, sorry on the very long delay here, I'm trying to get back to maintaining this gem actively and am resolving or closing most open issues.

I messed up on the backwards compatibility here, that's my fault. I'll be more careful going forward but I think given the delay this ship has sailed, if I change it again that'll break people on the newer versions, so I'm going to close this out

arkadiyt avatar Sep 01 '23 18:09 arkadiyt

Yeah I can understand your decision. Thank you for the reply.

mshibuya avatar Sep 02 '23 14:09 mshibuya