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

restrict botflagged hosts to engine versions available on https://sp… #261

Closed
wants to merge 1 commit into from
Closed

Conversation

silentwings
Copy link
Contributor

…ringrts.com/dl/buildbot

@silentwings
Copy link
Contributor Author

attempt to fix #260

my first Python code, who knows what I broke

@gajop
Copy link
Member

gajop commented Jun 1, 2018

doing a HTTP request each time someone tries to host a battle seems very inefficient

@silentwings
Copy link
Contributor Author

silentwings commented Jun 2, 2018

very inefficient

Could you be more specific? I would have thought making a single/double digit number of https reqs per day had a cost of approximately zero, but I am not experienced here...

You prefer to cache recently checked urls, to handle maliciously spammed requests, or something? It's not obvious to me how to avoid blocking while the url is checked.

@specing
Copy link

specing commented Jun 2, 2018

Can this be extended to games and maps as well? And to all hosts, not just botflagged ones. If you want to test something you can use the test lobby server.

@silentwings
Copy link
Contributor Author

silentwings commented Jun 2, 2018

Can this be extended to games and maps as well?

In theory, of course, with more effort, but in this pull request: no.

In practice: no. Auto-dl will never be mandatory for content. The role of the url check here is that it matches when engine debug symbols are stored/deleted, so even users with access to outdated dev engines should should stop using them.

to all hosts, not just botflagged ones

No - such restrictions won't ever be imposed on private battle hosts. Doing so would prevent devs and interested players from developing and testing new content.

(Also, afaik, the test lobbyserver is for testing commits after they have been committed, and doesn't cover PRs. My local uberserver should be fine.)

@specing
Copy link

specing commented Jun 2, 2018

It would not prevent development as there is still the test lobby server that can be used for this. And your pr does not check whether a room is private(=passworded) or not. This would also help test new versions of the lobby server and make newbie experience a bit less frustrating.

@silentwings
Copy link
Contributor Author

silentwings commented Jun 2, 2018

A quick glance at reality will tell you that game/map/etc development does not take place on the test lobbyserver, and a quick bit of thinking will tell you that nor will it ever. The test lobbyserver is for testing the lobbyserver.

Private host != passworded host. Private host = not botflagged host. Passworded = passworded. Any engine version control will apply only to botflagged hosts.

Please refrain from these off-topic comments, or make them elsewhere.

@gajop
Copy link
Member

gajop commented Jun 3, 2018

My previous inefficiency comment arises from the fact that you'd do this check within the OPENBATTLE command, making it much slower, and possibly undefined in case it times out due to some network issues.

Ideally, if you want to do something like this, it should be a background script that updates the same database. (Like abma previously suggested)

@silentwings
Copy link
Contributor Author

silentwings commented Jun 3, 2018

The timeout issue is a problem, I can't see a non-exploitable way to do the check in real-time, and this conflicts with the requirement that new dev versions should be available for use near instantly. Atm not sure how to do it; perhaps a re-check once per hour would be enough (?).

Another issue is that, if not checking a particular version string on request, there seems no clean way to obtain a list of currently dl-able spring versions: https is not suitable for dir listing. I guess buildbot needs to record this info into a file, somewhere (?).

There isn't much point in storing this data in the database; updates are needed regularly.

@silentwings
Copy link
Contributor Author

Will rethink... but answers to above qs welcome, if you know them.

@silentwings silentwings closed this Jun 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants