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.

Comments

Yaron Tal created an issue. See original summary.

Yaron Tal’s picture

StatusFileSize
new1.58 KB
Yaron Tal’s picture

Status: Active » Needs review
mrdalesmith’s picture

Cheers 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?

Yaron Tal’s picture

Sounds 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.

Yaron Tal’s picture

StatusFileSize
new1.87 KB

New 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.

mrdalesmith’s picture

StatusFileSize
new3.28 KB

Cheers @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?

Yaron Tal’s picture

I 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.

mrdalesmith’s picture

That'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.

Yaron Tal’s picture

If 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).

mrdalesmith’s picture

StatusFileSize
new2.58 KB

That'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?

Yaron Tal’s picture

Looks 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?

Yaron Tal’s picture

Looks 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.

  • MrDaleSmith committed b4c98e9 on 7.x-3.x authored by Yaron Tal
    Issue #2867311 by Yaron Tal, MrDaleSmith: Also fix webforms that use the...
mrdalesmith’s picture

Status: Needs review » Fixed

The 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.

mrdalesmith’s picture

Status: Fixed » Closed (fixed)