Problem/Motivation

When submitting the admin form without 'Mapped path' the following php notice is given:

Warning: Invalid argument supplied for foreach() in adform_admin_settings_form() (regel 27 van /var/www/eyefilm/htdocs/sites/all/modules/contrib/adform/adform.admin.inc).

Further I noticed that the code assumes "\n" as line separator.

Proposed resolution

Make the user input more bullet proof.

CommentFileSizeAuthor
#6 make_user_input-2875158-6.patch1.54 KBxen
#4 make_user_input-2875158-4.patch2.13 KBxen
#2 adform-admin-input-2875158-2.patch1.84 KBsutharsan

Comments

Sutharsan created an issue. See original summary.

sutharsan’s picture

sutharsan commented
Title: Prevent » Make user input handling bullet proof
StatusFileSize
new adform-admin-input-2875158-2.patch1.84 KB
sutharsan’s picture

sutharsan commented
Status: Active » Needs review
xen’s picture

xen commented
StatusFileSize
new make_user_input-2875158-4.patch2.13 KB

Heh, funnily enough the issue was caused by a patch that tried to fix an issue that was probably caused by \r in the input.

But i think the issue can be fixed lighter, please try the attached patch.

sutharsan’s picture

sutharsan commented
Status: Needs review » Reviewed & tested by the community

Patch works fine.

However, I would not change the Hostnames separator behaviour, "... seperated by space or on seperate lines." (or not in this patch). When saving the hostnames, the space separator is replaced by a new line.

xen’s picture

xen commented
StatusFileSize
new make_user_input-2875158-6.patch1.54 KB

OK, stepping back a minute. Have you actually run into issues with \r? No modern OS uses only \r for newline, and the trimming of the exploded lines in the original code should strip out extra \r making special handling unnecessary.

Try the attached patch which reverts the original "fix", especially if it handles an empty mapping OK.

sutharsan’s picture

sutharsan commented

No, I have not ran into problems with \r. Apparently I'm repeating an old pattern, also in core (e.g. RequestPath) only \n is used.

The reason I worked on this code was to fix the problem that adform_mappings is stored as string (and not as empty array) when no mappings are entered.

RTBC again, thanks.

xen’s picture

xen commented
Status: Reviewed & tested by the community » Fixed

Fix committed. Release 1.2 should be available shortly.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.