MantisBT - Zandronum
View Issue Details
0001194Zandronum[All Projects] Bugpublic2012-12-01 04:402015-03-16 01:25
Metal 
Edward-san 
immediatemajoralways
closedunable to reproduce 
LinuxUbuntu10.04 x86
 
1.41.4 
0001194: Banlists not working
On servers such as BE, NJ and sometimes GV, the banlist does not add entries or erases them quickly. Votekicks do not ban for 10 minutes. tested on Windows 7 machine and works fine. AFAIK, NJ, GV, and BE are hosted on Linux.
1. Make a server (On linux)
2. Ban yourself or another player
3. You can rejoin without executing delban or removing the ban yourself.

I'm unsure if this problem is actually random or not. But I do know some server hosts have to manually go in to their server directories to add bans.
No tags attached.
patch q.patch (6,200) 2015-02-07 18:55
/tracker/file_download.php?file_id=1409&type=bug
txt banlist.diff.txt (2,681) 2015-02-08 01:48
/tracker/file_download.php?file_id=1410&type=bug
Issue History
2012-12-01 04:40MetalNew Issue
2012-12-01 15:36DuskNote Added: 0005450
2012-12-01 15:36DuskStatusnew => feedback
2012-12-01 19:42Torr SamahoNote Added: 0005451
2012-12-02 01:34MetalNote Added: 0005453
2012-12-02 01:34MetalStatusfeedback => new
2012-12-02 01:35MetalNote Edited: 0005453bug_revision_view_page.php?bugnote_id=5453#r2989
2012-12-02 01:36MetalNote Edited: 0005453bug_revision_view_page.php?bugnote_id=5453#r2990
2012-12-02 01:36MetalNote View State: 0005453: private
2012-12-02 01:36MetalNote View State: 0005453: public
2012-12-02 01:39MetalNote Edited: 0005453bug_revision_view_page.php?bugnote_id=5453#r2991
2012-12-02 01:48MetalNote Edited: 0005453bug_revision_view_page.php?bugnote_id=5453#r2992
2012-12-02 01:50MetalNote Edited: 0005453bug_revision_view_page.php?bugnote_id=5453#r2993
2012-12-02 08:19Torr SamahoNote Added: 0005454
2012-12-02 21:44MetalNote Added: 0005462
2012-12-02 22:06Torr SamahoNote Added: 0005463
2012-12-03 23:50MetalNote Added: 0005470
2012-12-04 06:15Torr SamahoView Statusprivate => public
2012-12-04 06:15Torr SamahoNote Added: 0005471
2012-12-04 19:15Konar6Note Added: 0005473
2012-12-04 19:16Konar6Note Edited: 0005473bug_revision_view_page.php?bugnote_id=5473#r2998
2012-12-05 21:02WatermelonNote Added: 0005477
2012-12-06 02:05AlexMaxNote Added: 0005478
2012-12-06 02:07AlexMaxNote Edited: 0005478bug_revision_view_page.php?bugnote_id=5478#r3002
2012-12-06 02:08AlexMaxNote Edited: 0005478bug_revision_view_page.php?bugnote_id=5478#r3003
2012-12-07 18:49Torr SamahoNote Added: 0005480
2012-12-08 01:40AlexMaxNote Added: 0005484
2012-12-08 14:30Konar6Note Added: 0005487
2013-01-13 10:40Torr SamahoNote Added: 0005758
2013-01-13 10:40Torr SamahoStatusnew => feedback
2013-01-15 06:17DevilHunterNote Added: 0005785
2013-08-17 00:06CatastropheNote Added: 0007034
2013-08-17 04:55DuskNote Added: 0007037
2013-08-17 04:56DuskNote Edited: 0007037bug_revision_view_page.php?bugnote_id=7037#r3951
2013-08-17 09:14Konar6Note Added: 0007038
2013-08-17 10:05Torr SamahoNote Added: 0007040
2013-08-17 10:27Torr SamahoNote Deleted: 0007040
2014-11-22 05:49DevilHunterNote Added: 0010928
2015-02-07 18:55Konar6File Added: q.patch
2015-02-07 18:56Konar6Note Added: 0011611
2015-02-07 19:03DuskStatusfeedback => needs review
2015-02-07 20:29Edward-sanNote Added: 0011613
2015-02-07 20:34Konar6Note Added: 0011614
2015-02-08 01:48Edward-sanNote Added: 0011621
2015-02-08 01:48Edward-sanFile Added: banlist.diff.txt
2015-02-08 09:41Torr SamahoNote Added: 0011622
2015-02-08 10:19Edward-sanNote Added: 0011623
2015-02-08 12:35Torr SamahoNote Added: 0011627
2015-02-08 12:40Torr SamahoNote Edited: 0011627bug_revision_view_page.php?bugnote_id=11627#r6593
2015-02-08 12:41Konar6Note Added: 0011628
2015-02-08 12:48DuskNote Added: 0011629
2015-02-08 12:49DuskNote Edited: 0011629bug_revision_view_page.php?bugnote_id=11629#r6595
2015-02-08 12:55Torr SamahoNote Added: 0011630
2015-02-08 13:36Edward-sanNote Added: 0011632
2015-02-08 15:07Edward-sanNote Edited: 0011632bug_revision_view_page.php?bugnote_id=11632#r6597
2015-02-15 20:18cobaltStatusneeds review => needs testing
2015-02-15 20:18cobaltTarget Version => 1.4
2015-02-15 20:18cobaltSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=6649#r6649
2015-02-15 20:18cobaltNote Added: 0011691
2015-02-15 20:54Torr SamahoNote Added: 0011693
2015-02-15 21:31Konar6Note Added: 0011695
2015-02-15 21:32Konar6Note Edited: 0011695bug_revision_view_page.php?bugnote_id=11695#r6651
2015-02-15 22:38Edward-sanNote Added: 0011696
2015-03-16 01:25DuskNote Added: 0011834
2015-03-16 01:25DuskStatusneeds testing => closed
2015-03-16 01:25DuskAssigned To => Edward-san
2015-03-16 01:25DuskResolutionopen => unable to reproduce
2015-03-16 01:25DuskFixed in Version => 1.4
2015-03-16 01:25DuskSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=6785#r6785

Notes
(0005450)
Dusk   
2012-12-01 15:36   
Works fine for me under Debian...
(0005451)
Torr Samaho   
2012-12-01 19:42   
I can't reproduce these problems either. Did this only recently start happening? Are the affected servers running the official Zandronum 1.0 binary or some modified version?
(0005453)
Metal   
2012-12-02 01:34   
(edited on: 2012-12-02 01:50)
Yes, this started fairly recently. AFAIK, they are running modified binaries. NJ is the only one using the Zandro 1.0 binary. But they still encounter this problem.

Edit: I kind of figured it was only on linux because I tested it on my own windows machine and it seemed to work perfectly. Sorry for the confusion.

A small log from another person experiencing this problem.
[21:38] <Watermelon> if it helps, when you press "up" on your keyboard to do the last command, this is what happened to me
[21:38] <Watermelon> oh damn didnt know that
[21:38] <Watermelon> addban "ip" "perm" "reason"
[21:38] <Watermelon> *press up to get the above line again*
[21:38] <Watermelon> addban "ip" "perm" "reason"
[21:38] <Watermelon> i did that every second
[21:39] <Watermelon> for the first 5 seconds (5 ups and enters) it says "ip is already on the ban list"
[21:39] <Watermelon> then randomly it gets removed after 5 seconds

(0005454)
Torr Samaho   
2012-12-02 08:19   
Is the IP ban file opened by any other program while the servers are running?
(0005462)
Metal   
2012-12-02 21:44   
They say no.
(0005463)
Torr Samaho   
2012-12-02 22:06   
Is the file write protected? If this didn't happen earlier, something has to have changed on their systems.
(0005470)
Metal   
2012-12-03 23:50   
The host for BE says

[19:49] <Jenova> no
[19:49] <Jenova> and this has been a problem forever

I'm not getting a straight answer from anyone. Watermelon (The co host of BE) says it's been recent, and Jenova says it's been forever.

[19:49] <Jenova> like since I first installed st on the box
[19:50] <Jenova> it can add to the file
[19:50] <Jenova> but its messed up

I think it would be beneficial to open this ticket to the public so they can reply themselves.
(0005471)
Torr Samaho   
2012-12-04 06:15   
Ok, the ticket should be public now.
(0005473)
Konar6   
2012-12-04 19:15   
(edited on: 2012-12-04 19:16)
There are two independent issues mentioned in this ticket.

First one isn't really an issue in Zandronum - NJ is using a modified binary, and Watermelon confirmed that it was the cause, as the official build worked fine (and apparently he fixed his one too now).

The second issue occuring on Jenova's server is a duplicate of'http://zandronum.com/tracker/view.php?id=290 [^]' , which reminded me that I had that problem back in 2010 on one of my servers before I switched it to Ubuntu (it affects only some systems - Debian 64bit being one). Now when the source is open, I inspected it again and here comes the fix. Unlike my expectations, no change in the source was needed to make it work. Just build Zandronum from source on the system and that's it. Or here is a precompiled working 1.0 server binary, built on the "problematic" Debian 64bit system ->'http://sickedwick.net/misc/zandronum-server_debian_x64.zip [^]'

(0005477)
Watermelon   
2012-12-05 21:02   
Interestingly as Konar said, the regular build seemed to have worked. I compiled a new version fresh and terminated all the servers.

Though it seems that the problem begins to happen later on, so I will ideally stay tuned. Resetting the servers could be the key to this potential problem. For some reason when I recompiled it fixed itself... which doesn't make sense since I never touched the ban mechanisms in the first place. Could there be possibly something that causes it to go hay-wire later on?
(0005478)
AlexMax   
2012-12-06 02:05   
(edited on: 2012-12-06 02:08)
For what it is worth, I have been having this problem with bans since Zandronum 1.0. I do not run a modded server (Spak City runs wbuild, but Spak City and Funcrusher are not affiliated), just the binary grabbed from zandronum site. I load the banlist on a timer, and what I find in the banlist after commands are run is...interesting.

From my limited experimentation with "addban", it seems like any permanent ban is written correctly. Also, updating an existing ban in any case works fantastically. However, new timed bans are where things mess up...if you ban someone for say 30min for no reason, only the ban time will be written to the file. If you add a new timed ban with a reason, only the colon and the reason are written to the file. This applies to votebans too.

So it seems to me that as a workaround, admins should permaban people first, then update the ban after-the-fact to a specific time.

(0005480)
Torr Samaho   
2012-12-07 18:49   
Quote from AlexMax
From my limited experimentation with "addban", it seems like any permanent ban is written correctly. Also, updating an existing ban in any case works fantastically. However, new timed bans are where things mess up...if you ban someone for say 30min for no reason, only the ban time will be written to the file. If you add a new timed ban with a reason, only the colon and the reason are written to the file. This applies to votebans too.

I just tested this briefly, but addban works fine for me with non-permanent bans. Who can confirm that there are problems with addban? And for those who have problems: Does it also happen if Zandronum itself created the banlist file and no other program ever touched that file?
(0005484)
AlexMax   
2012-12-08 01:40   
Torr, I tried adding a ban to a fresh banlist, and it had the same effects.
(0005487)
Konar6   
2012-12-08 14:30   
Did you read/try the solution I posted above?
(0005758)
Torr Samaho   
2013-01-13 10:40   
Without more information there is nothing I can do. It works just fine for me on all systems I tested.
(0005785)
DevilHunter   
2013-01-15 06:17   
C4 tried to make a Static Link of Zandronum, but for some reason could not find a way to.

[08:51:21] [Affliction] a makefile without any references to any form of linker settings :|
[08:55:54] [Affliction] so, it compiles fine
[08:56:19] [Affliction] But unless I can find some way to statically link it, we won't be able to rule out the alleged issue as being my server's fault
[09:06:46] [Affliction] Well, I don't see any form of linker settings anywhere. No point compiling it to correct this alleged environment issue if it can't be static linked.
[10:47:05] [Affliction] I can compile it fine, I just can't figure out how to modify the (c)makefile to pass a very simple argument to the linker
(0007034)
Catastrophe   
2013-08-17 00:06   
Please fix this already there are people openly evading kicks on [NJ] Funcrusher, this needs to be fixed -immediately-
(0007037)
Dusk   
2013-08-17 04:55   
(edited on: 2013-08-17 04:56)
And how is this supposed to be fixed when nobody can reproduce this? Please don't bump tickets without adding anything new to it.

(0007038)
Konar6   
2013-08-17 09:14   
Well, I reproduced this and found a (immediate) solution above. Just have the admin of NJ apply it...
(0010928)
DevilHunter   
2014-11-22 05:49   
I know this is old. But I somewhat found a fix for this.. I just gone ahead, and grabbed the source, and built it on TSPG, and Armada. The bans work as they should.

I just can't figure out for the life of me, why the precompiled builds on the site have this issue.
(0011611)
Konar6   
2015-02-07 18:56   
Fixed in the attached patch.
(0011613)
Edward-san   
2015-02-07 20:29   
It's not clear if the issue happens in 2.0 beta builds. Please check this out.

If it happens in the beta builds, it's gonna be easier to check the fix, ie make Blzut3 build a special zandronum x64 build containing the fix.
(0011614)
Konar6   
2015-02-07 20:34   
DevilHunter: ^
(0011621)
Edward-san   
2015-02-08 01:48   
I've taken a look at the patch and I believe there's no need for FString, hence I've made a somewhat simpler patch which should do the job fine.
(0011622)
Torr Samaho   
2015-02-08 09:41   
Before I look at the patches, can somebody summarize what the problem is that causes the ban list not to work on some systems?
(0011623)
Edward-san   
2015-02-08 10:19   
Torr, while it's not sure this is the real fix, but according to C and C++ rules, using "sprintf(str, "%s%s", str, other_str);" is undefined behavior, because of the memory overlapping. It was used in abundance in 'IPList::AddEntry' and in 'IPList::removeExpiredEntries'.

IMO in some systems such "sprintf" call wouldn't work at all, hence the "other_str" would be not appended to the existing "str", hence making the banlist entry adding not working properly.
(0011627)
Torr Samaho   
2015-02-08 12:35   
(edited on: 2015-02-08 12:40)
Can you point me to a source that describes the corresponding C/C++ limitations? Would make sense if this stuff doesn't have to work, but I'd like to understand what we are fixing before marking something as fixed in the history file.

(0011628)
Konar6   
2015-02-08 12:41   
"The behavior of this function is undefined if copying takes place between objects that overlap—for example, if s is also given as an argument to be printed under control of the ‘%s’ conversion."

'http://www.gnu.org/software/libc/manual/html_mono/libc.html#Formatted-Output-Functions [^]'

'http://stackoverflow.com/questions/1283354/is-sprintfbuffer-s-buffer-safe [^]'
(0011629)
Dusk   
2015-02-08 12:48   
(edited on: 2015-02-08 12:49)
I'll just point out that, by the looks of it, FString::Format is not affected by this since it reallocates the Chars buffer before doing the format. FString::AppendFormat is, however, since it reallocates on demand (though you shouldn't need to pass the FString itself into AppendFormat anyway since you're already appending to it).

(0011630)
Torr Samaho   
2015-02-08 12:55   
Thanks for the detailed info! We should definitely change this and I agree with Edward-san that we should not start using FStrings for this, but use proper sprintf calls instead. This way the code doesn't get any additional ZDoom dependencies and can still easily be used in non-ZDoom code like the master server.

Edward-san, I have some comments to your patch. Can you commit it to the sandbox, then I can write the comments into the code changes on bitbucket.
(0011632)
Edward-san   
2015-02-08 13:36   
(edited on: 2015-02-08 15:07)
Done.

(0011691)
cobalt   
2015-02-15 20:18   
Issue addressed by commit 0cf13db5ce64: - Fixed: On some systems, adding bans and ban exemptions from the server console didn't work (fixes 1194).
Committed by edward_san [edward-san] on Sunday 15 February 2015 13:00:29

Changes in files:
 docs/zandronum-history.txt | 1 +
 src/networkshared.cpp | 83 ++++++++++++++++++++++++++++++++++++-----------------------------------------------
 2 files changed, 37 insertions(+), 47 deletions(-)
(0011693)
Torr Samaho   
2015-02-15 20:54   
I just hear that nobody ever confirmed that this patch actually fixes the problem. Konar6, is this true?
(0011695)
Konar6   
2015-02-15 21:31   
(edited on: 2015-02-15 21:32)
I don't know how we could accurately confirm that until the next beta build is released, since the bug can be reproduced only in the official builds.

(0011696)
Edward-san   
2015-02-15 22:38   
DevilHunter doesn't reproduce the issue with the official 1.3 build from this link:'http://zandronum.com/downloads/zandronum1.3-linux-x86_64.tar.bz2 [^]'

If someone else can reproduce the problem with that build, please reply.
(0011834)
Dusk   
2015-03-16 01:25   
Nobody can reproduce the problem anymore so.. let's just hope the commit fixed it I guess.