search

FriendsOfFlarum / webhooks

Flarum with outgoing webhooks
MIT License
21 stars 12 forks source link

Change Slack adapter URL preg_match pattern #14

Closed taddis closed 4 years ago

taddis commented 4 years ago

…cters. Slack URLs second subpattern can contain at least 11 characters. This future proofs for any length.

dsevillamartin commented 4 years ago

Any reason why you changed it from any character to only A-Z & 0-9? Wouldn't that be the opposite of future-proofing it?

taddis commented 4 years ago

Sorry, my phrasing wasn't on point :) But I was saying it specifically in regards to the length, nothing else. From what I understand it will "always" be uppercase alphanumeric characters for Team/Workspace ID and Service ID, therefore the change to [A-Z0-9], but the length can grow.

My communication with the Slack team:

Hi Tobias,

Thank you for getting in touch with us.

At this time, the Team ID (workspace ID) and Service ID are alphanumeric strings that will start with a T and a B. They do not have set lengths, but are at least 9 characters long. The character length was recently changed, actually. Per the Changelog (https://api.slack.com/changelog), so be aware that we can't guarantee that changes won't be made in the future:

The IDs returned by our APIs have grown longer. They are now up to 11 characters long, and could grow longer in future. Please audit your Slack apps, and verify any assumptions about the length of IDs for users, channels, and other objects.

Hope that helps. Let me know if there's anything else I can clarify.

taddis commented 4 years ago

I changed it back to handle any character now. But maybe the best option would be to have, based on what they communicated back where they didn't explicitly say uppercase, like this:

^https?:\/\/hooks\.slack\.com\/services\/T[a-zA-Z0-9]{8,}\/B[a-zA-Z0-9]{8,}\/.{24}$

Decide how you want it and I'll fix it.

dsevillamartin commented 4 years ago

To be honest, it doesn't really matter that much - the URL will either be valid or not. I think simply increasing the length is good enough.

Do they always start with T or B? Where does it say this?

taddis commented 4 years ago

To be honest, it doesn't really matter that much - the URL will either be valid or not. I think simply increasing the length is good enough.

My thought as well. But when I saw that you had a more complex regex then just checking for valid URL I thought that it did matter. But if it doesn't, discard this PR and do as you like.

Kind regards.

dsevillamartin commented 4 years ago

Well, there's a pro version of this package that allows sending data to any URL, so that's why I'm locking the services to their corresponding URLs. I will merge this. Thanks!