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

[WIP]Use useragent module to keep it updated #6444

Closed
wants to merge 2 commits into from

Conversation

oppilate
Copy link
Contributor

该 PR 相关 Issue / Involved issue

Some hard-code useragents are so old that became rare for real users. Relying on modules is more sustainable to keep up with top browser fingerprints.

@vercel
Copy link

vercel bot commented Dec 14, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/diy/rsshub/nenxcnuxz
✅ Preview: https://rsshub-git-fork-oppilate-useragent.diy.vercel.app

@HenryQW
Copy link
Collaborator

HenryQW commented Dec 15, 2020

I think we can implement a default UA in the middleware.

In config we can have ua.desktop and ua.mobile

@oppilate
Copy link
Contributor Author

I think we can implement a default UA in the middleware.

There's already a config.ua in the encapsulated got util. But the point here is, it's hard to keep the UA up-to-date manually. And using a hard-coded UA will make it easy to be identified.

In config we can have ua.desktop and ua.mobile

It's more than that. Some require Windows/MacOS/Android/iOS, and others require Chrome/Webkit/Firefox. Enumerate all of them will be equavelant to reinventing the user-agents module.

@HenryQW
Copy link
Collaborator

HenryQW commented Dec 18, 2020

I guess what I was trying to say was, creating a user-agents helper, I'm not sure if got.extend takes any parameters

RSSHub/lib/utils/got.js

Lines 33 to 35 in 0be537f

headers: {
'user-agent': config.ua,
},

In routes it will be convenient with the helper:

const response = await got({
        url,
        method: 'get',
        headers: {
            'User-Agent': UAHelper('chrome','desktop'),
        },
    });

@oppilate
Copy link
Contributor Author

In routes it will be convenient with the helper:

Sure. But I don't see many differences than user-agents? Except the brackets.

const response = await got({
    url,
    method: 'get',
    headers: {
        'User-Agent': (new UserAgent([
            /Chrome/,
            {
                deviceCategory: 'desktop',
            },
        ])).toString(),
    },
});

@HenryQW
Copy link
Collaborator

HenryQW commented Dec 23, 2020

const UA = require('@/utils/user-agent');

const response = await got({
        url,
        method: 'get',
        headers: {
            'User-Agent': UA.get('chrome','desktop'),
        },
    });

Where @/utils/user-agent handles the UA string generation. IMO this is more handy, what do you think?

Sorry I'm too busy to respond swiftly, multiple deadlines...

@oppilate
Copy link
Contributor Author

You are awesome ☺️. Just like you, I've finished my endless deadlines of this year just now.

For the problem, I'm convinced. I think a wrapper will make it cleaner, although there's no performance improvement (each time you still have to initialize a UserAgent instance). Just concerning about whether it would be type hinting unfriendly (especially useful in IDEs) as the got wrapper. Maybe there are some way to not have this drawback. Will look into it tomorrow.

@oppilate
Copy link
Contributor Author

got's type inference failure is a bug of itself: sindresorhus/got#1137. So wrappers are fine.

@oppilate oppilate changed the title Use useragent module to keep it updated [WIP]Use useragent module to keep it updated Dec 25, 2020
@HenryQW
Copy link
Collaborator

HenryQW commented Dec 26, 2020

The random UA json is outdated with iOS 11 as the latest...

@oppilate
Copy link
Contributor Author

yes... Still looking for a module with an up-to-date database..

@HenryQW HenryQW marked this pull request as draft December 28, 2020 17:24
@NeverBehave
Copy link
Collaborator

I think we could still have it, and wrap a little bit in case we need to switch to other package. It will definitely better than our existing one. Please advise.

@TonyRL
Copy link
Collaborator

TonyRL commented Apr 2, 2022

Superseded by #9449

@TonyRL TonyRL closed this Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants