search

reconquest / atlassian-external-hooks

External Hooks plugin for Atlassian Bitbucket
https://external-hooks.reconquest.io
Other
44 stars 37 forks source link

Changed the hook response to REJECT the push when output exceeds 65K #8

Closed rappazzo closed 10 years ago

rappazzo commented 10 years ago

As I stated in my comment to the parent changeset, I believe that returning true when the output exceeds 65K is a bug (and it could lead to change sets which violate what the hooks are designed to keep out).

If you think that I am wrong about this, I have also coded a change that would allow the hook setup to specify what occurs when there is excessive output. In that case, please reject this pull request, and indicate that I should make a pull request for that change set.

seletskiy commented 10 years ago

Ok, I think hook should always return false if output limits are exceeded.

But clipping output should not be an option, because of if output exceeds 65K, then Stash silently aborts a push with non-descriptive error like "communication error".

seletskiy commented 10 years ago

Please, bump "release" component of version in pom.xml so I can merge a PR and deploy hook on marketplace.

rappazzo commented 10 years ago

Done.

rappazzo commented 10 years ago

So, now a side effect of this change is that when you have a pre-receive hook that is normally non-rejecting, but produces excessive output, it will reject.

If you prefer, I can modify my other change to always truncate/clip the output, and then have an option to discard further output or reject the push. Let me know which you think is best.

seletskiy commented 10 years ago

Thanks for the contribution! I've uploaded new version to the marketplace.

seletskiy commented 10 years ago

Regarding your comment about side-effect: situation in which hook produces over 65K over information is abnormal and should be avoided. So, I think it's good to forbid push unconditionally.

rappazzo commented 10 years ago

Thanks. I appreciate that you are responsive and that we can easily collaborate on this. On May 13, 2014 7:41 AM, "Stanislav Seletskiy" notifications@github.com wrote:

Regarding your comment about side-effect: situation in which hook produces over 65K over information is abnormal and should be avoided. So, I think it's good to forbid push unconditionally.

— Reply to this email directly or view it on GitHubhttps://github.com/ngsru/atlassian-external-hooks/pull/8#issuecomment-42944553 .