Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create user-agent constant SMF_USER_AGENT #5706

Open
wants to merge 6 commits into
base: release-2.1
from

Conversation

Projects
None yet
5 participants
@MissAllSunday
Copy link
Contributor

commented Jun 5, 2019

This creates a new constant to define our own user agent

#5670

Create user-agent constant SMF_USER_AGENT
Signed-off-by: Jessica González <suki@missallsunday.com>

@sbulen sbulen added this to the Final milestone Jun 6, 2019

@albertlast

This comment has been minimized.

Copy link
Collaborator

commented Jun 8, 2019

Why hard code this ua than?
Would it be not better so save this in the settings?

To use ua for the stats collection issue,
i don't like.

@MissAllSunday

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

The user agent isn't something we want to change, a setting is mainly used for things that we want to change at some point in time, since we don't want to change the UA and a PHP constant by definition prevents changes, a constant seems a better choice.

Show resolved Hide resolved index.php Outdated

MissAllSunday added some commits Jun 13, 2019

Define SMF_USER_AGENT on other required files
Signed-off-by: Jessica González <suki@missallsunday.com>
Set define('SMF_USER_AGENT', 'Mozilla/5.0 (' . php_uname('s') . ' ' .…
… php_uname('m') . ') AppleWebKit/605.1.15 (KHTML, like Gecko) PHP/' . PHP_VERSION . ' SMF/' . strtr(SMF_VERSION, ' ', '.')); as user agent

Signed-off-by: Jessica González <suki@missallsunday.com>
split user-agent: from constant
Signed-off-by: Jessica González <suki@missallsunday.com>
@albertlast
Copy link
Collaborator

left a comment

The request create new issues,
like sending server information across to different sites and
also smf version with no needs.

Also the user agent could be blocked on some pages,
which would break the proxy logic,
test would be needed but no one is able to provide.

@Sesquipedalian

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

On further consideration, I suppose we should drop the PHP version info out of it. It isn't relevant information for a UA, and it's the one part that could conceivably have any sort of security implications. Concerns about the rest of the info are unfounded.

If you are concerned that servers won't like this UA, @albertlast, you can easily test it for yourself against any servers you like. Just use your browser's custom user agent feature or curl's --user-agent option on the command line to test as many URLs as you want. If you find any sites that object to Mozilla/5.0 (Linux x86_64) AppleWebKit/605.1.15 (KHTML, like Gecko) SMF/2.1.RC2, by all means feel free to come back and tell me how wrong I have been. But in my tests I haven't found a single one that behaved any differently with that UA than they did if I connected using a standard Chrome or Safari UA.

FYI, the general pattern for WebKit-based browsers is Mozilla/5.0 (<system and browser information>) AppleWebKit/<version> (KHTML, like Gecko) <extended info>. Any server that sees a UA matching this pattern is going to accept that they are dealing with some sort of WebKit-based browser.

@Sesquipedalian

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

@MissAllSunday, let's go ahead and remove the PHP version info part after all. Since our own stats collection doesn't care about the UA, including it gains us nothing and we may as well not bother. Let's keep the SMF version part, though, since that could conceivably be useful info for various reasons.

michel.mendiola added some commits Jun 24, 2019

Jessica González Jessica González
Remove php info
Signed-off-by: Jessica González <suki@missallsunday.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.