When entering something like "-1" or "" into the port field at "admin/settings/apachesolr", submitting shows a WSoD, as does e.g. the "Status Reports" page (admin/reports/status) or probably any other page which tries contacting the Apache Solr server.
Two points:
*) Validate form input properly (valid host data) before saving it.
*) Invalid host data should result in an error message, not a WSoD.
The first I will probably take care of, soon, should be done quickly.
PS: Especially bad about this is that you then have no possibility to reset the port the normal way, since the settings page doesn't get displayed. I had to use devel's "Execute PHP" block, normal users who accidentally set this to a bad value won't be too glad with that.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | apachesolr-543226-6.patch | 1.61 KB | janusman |
| #5 | port_validation.patch | 1.56 KB | drunken monkey |
| #1 | port_validation.patch | 1.6 KB | drunken monkey |
Comments
Comment #1
drunken monkeyAdditional information:
* Port numbers above 65535 are no problem, but 0 (which is theoretically allowed) also leads to a WSoD (= "White Screen of Death", by the way – just realized I used it without once explaining it).
* The host and path fields may contain literally every type of garbage, including tabulator, arbitrary Unicode characters, etc. – but the host has to be non-empty.
The attached patch sets host and port fields to "required" and will validate the port number to be an integer between 1 and 65535.
Comment #2
pwolanin commentedpatch looks reasonable
Comment #3
robertdouglass commentedCan you use is_int() instead of doing the cast?
Comment #4
robertdouglass commentedComment #5
drunken monkeyAs form values are always string values, even when specifying valid numbers, this would always return FALSE.
But ctype_digit() seems to be exactly what we want, so corrected this in the attached patch. It really does look a lot nicer that way. ;)
Comment #6
janusman commentedPatch applies with offset against current 6.x-2.x-dev. Attached patch is just a fresh diff from that.
Comment #7
robertdouglass commentedFixed in 6.1 and 6.2. Thanks.