|
Tim Hunt made changes - 15/Jul/08 10:02 PM
Full ip address restriction in quiz
ex.: quiz --> Require network address: 192.168.10.10 It will be interpreted as type 2 and will accept 192.168.10.10 and so 192.168.10.100 to 192.168.10.109 So we asked our teachers to use type 1 by adding a '/' at the end. The thing I don't remember is why we didn't ask them to add '/32'. Did it came from the doc? Note that the problem was treated on moodle 1.6.3 in 2007. So, the difference on 64 bit appears when ending with '/' This will work well on 32 bit cpu but will not work on 64 bit because of this instruction: On 32 bit, no shift will be done and $mask will get 0xFFFFFFFF Conclusion: After many tests and research, I suggest to ignore this patch and add '/32' for full ip addresses. The doc need to be change or the code for type 2 when using full ip addresses need change. In this last case, may be using '*' for partial addresses would be a good solution. Thank you for the clear analysis and explanation. I think I understand now.
I did not realise that / was valid syntax. I guess that the best thing is to explicitly check for this case, and treat it as /32. Also, it was a bug if 192.168.10.10 matched 192.168.10.100. Type 2 should only apply to entire parts of the address, it should not be a general sub-string mechanism. I have added unit tests for these cases, and then changed the code so that they all pass. Hopefully that fixes the problem, however I don't have an easy way to test on a 64 bit architecture, so it would be good if you could. Thanks.
Tim Hunt made changes - 01/Sep/08 12:15 PM
tjhunt committed 2 files to 'Moodle CVS' on branch 'MOODLE_17_STABLE' - 01/Sep/08 12:17 PM
tjhunt committed 2 files to 'Moodle CVS' - 01/Sep/08 12:17 PM
tjhunt committed 2 files to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 01/Sep/08 12:17 PM
tjhunt committed 2 files to 'Moodle CVS' on branch 'MOODLE_18_STABLE' - 01/Sep/08 12:18 PM
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Before applying your fix, I would like to understand it better. At the moment I am confused. All we are doing with $mask is &ing it with some 32 bit integers, so why should it matter if there are some extra bits set in $mask?
Would it be possible for you to try running the unit tests for this function on your 64-bit CPU? You just need to go to the URL .../admin/report/simpletest/index.php?&path=lib/simpletest/testmoodlelib.php on your moodle server. Do all those tests pass, both with and without your change?
One think about your fix that I don't like is the way it uses string manipulation on $mask. Wouldn't it be better to do $mask = $mask & 0xffffffff; instead of your line of code?