When fixing webforms, the module looks for the string 's:6:"scheme";s:6:"public"' in the webform_components table. The problem is that in some cases this string is not there, while the field is still a public field that needs fixing.
The solution would be to check for components that don't have 's:6:"scheme";s:7:"private"', unserialize, set scheme to private and update the row.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | also_fix_webforms_that-2867311-9.patch | 2.58 KB | mrdalesmith |
| #7 | also_fix_webforms_that-2867311-7.patch | 3.28 KB | mrdalesmith |
| #6 | also_fix_webforms_that-2867311-6.patch | 1.87 KB | Yaron Tal |
| #2 | also_fix_webforms_that-2867311-2.patch | 1.58 KB | Yaron Tal |
Comments
Comment #2
Yaron Tal commentedComment #3
Yaron Tal commentedComment #4
mrdalesmith commentedCheers for this @yaron-tal: what I liked about the the original expression is that it would update all webforms in two DB calls. Your solution is more robust but is doing a lot more database work. What would you think to us running the existing code twice, once for the public scheme and once for the default scheme if the default isn't set to be private?
Comment #5
Yaron Tal commentedSounds good. I also got the comment from a colleague that there might be other schema's (like s3fs) which will be overwritten this way. I'll try to create a new patch.
Comment #6
Yaron Tal commentedNew patch that checks for (public scheme OR no scheme at all) to check for files, does public->private the old way and only unserializes and updates when no scheme was set.
Comment #7
mrdalesmith commentedCheers @yaron-tal - I was thinking of something more like this, so that we're not doing more work than we need to: if the default file scheme is private then we don't need to update anything. Does this patch suit your needs?
Comment #8
Yaron Tal commentedI tried setting my default scheme tot private, but it looks like webforms has "public" set by default, and does not use the Drupal-default. So even if the default filesystem is Private, an empty scheme in a webform component will still make the field public.
Comment #9
mrdalesmith commentedThat's going to be problematic then: that suggests that any new webform components the users create after this process have been run will still be insecure. I'll see if I can find any related instructions / issues in the webform queue.
Comment #10
Yaron Tal commentedIf one of the protect settings has been enabled, the public option won't be available. So all new components will be saved with the private option (or another non-public scheme).
Comment #11
mrdalesmith commentedThat'll be me not remembering what the module I wrote does :)
OK, amended patch so that it updates all files stored in the default area in a single transaction: look ok to you?
Comment #12
Yaron Tal commentedLooks good, but does it also work in other database servers than mysql?
I agree that a db_update per component sounds bad, but maybe we could use batches like with moving files to support huge numbers of components?
Comment #13
Yaron Tal commentedLooks like postgresql, mysql and sqlite use the exact same syntax, still not sure about bypassing the db abstraction layer, but I don't see any problems with support for those.
Comment #15
mrdalesmith commentedThe module doesn't really support non-MYSQL as the other functionality will also struggle if the DB doesn't support basic expressions: I think that would be a bigger bit of work to make it completely DB agnostic and do everything through batches. But yeah, as you say this kind of basic expression is pretty well supported.
It's not bypassing the DB abstraction layer - it's using it: it is bypassing the entity API and webform hooks though, which might be something for me to look at another time: it's a deliberate sacrifice to improve performance.
Comment #16
mrdalesmith commented