bugs fixed by mda:
1. A case of "flexinode/" instead of "flexinode-"
2. Two cases of creating bad sql by implode on empty selected fields.
3. flexinode_page_table not doing correct sql for default $ctype_id = 0.
4. stop using quotes in perms, because i end up with backslashes in the permission column, which won't match.
this is because fix_gpc_magic only changes values, not keys, which are necessary for checkboxes.
so magic_quotes escapes once, then db_query does again.
5. flexinode_page_table was not creating table columns for empty values, shifting column cells.
6. no error detection of a file upload error (for example exceeding size).
this fix involves changes to file.inc and common.inc which are distinct patches:
http://drupal.org/node/view/8430
http://drupal.org/node/view/8429
7. qualify saved files by nid, so users can't cause other user's files to get deleted.

My patches are at:
http://apacam.dyndns.org/flexinode.patch
http://apacam.dyndns.org/field_file.patch

CommentFileSizeAuthor
#5 flexinode.patch.txt12.3 KBmda-1
#4 field_file.patch.txt3.07 KBmda-1

Comments

mda-1’s picture

mda-1 commented
killes@www.drop.org’s picture

killes@www.drop.org commented

I get 404s for the patches. Please attach them to this node and set to patch again.

jonbob’s picture

jonbob commented

Some of these bugs are duplicates of other issues, others are already fixed, and the patch is a 404. Please open individual issues as needed, and attach patches to the issues to avoid the hosting problem.

mda-1’s picture

mda-1 commented
StatusFileSize
new field_file.patch.txt3.07 KB

Sorry for the 404. I've fixed the 404 (so the links work), and I'll also attach.

Splitting the patches and reading every single bug report against flexinode
is beyond my patience, sorry.

mda-1’s picture

mda-1 commented
StatusFileSize
new flexinode.patch.txt12.3 KB

and here is the other patch.

jonbob’s picture

jonbob commented

I'm not trying to be rude, here; your patch is appreciated. It's just extremely difficult to analyze a patch that addresses several disjoint issues, some of which are no longer applicable. In addition, the title of the bug report becomes meaningless, which causes problems for people who are reading other people's bug reports before submitting their own. An initial run-through of your various issues shows:

1. A case of "flexinode/" instead of "flexinode-"
Requires investigation.

2. Two cases of creating bad sql by implode on empty selected fields.
Requires investigation.

3. flexinode_page_table not doing correct sql for default $ctype_id = 0.
Does not happen in HEAD.

4. stop using quotes in perms, because i end up with backslashes in the permission column, which won't match.
this is because fix_gpc_magic only changes values, not keys, which are necessary for checkboxes.
so magic_quotes escapes once, then db_query does again.
Does not happen in HEAD.

5. flexinode_page_table was not creating table columns for empty values, shifting column cells.
Does not happen in HEAD.

6. no error detection of a file upload error (for example exceeding size).
this fix involves changes to file.inc and common.inc which are distinct patches:
http://drupal.org/node/view/8430
http://drupal.org/node/view/8429
Can't apply until the referenced patches are, at the least.

7. qualify saved files by nid, so users can't cause other user's files to get deleted.
Are you sure this is a bug? File.inc already ensures that filenames are unique and cannot overwrite one another.

mda-1’s picture

mda-1 commented

I just sync'd to latest cvs to do the comparison again.

1. gone now; forget it.

2. still one case:
$sql = 'SELECT n.title, n.nid, '. implode(', ', $fields_to_select) .' FROM {node} n '. implode(' ', $table_joins) ." WHERE n.type = '$type' AND ". implode(' AND ', $where_clauses);
Though this is related to item 3; it is unlikely to happen unless flexinode_page_table supports $ctype_id = 0

3. In cvs, it deals with this by simply making flexinode_page_table return not found if $ctype_id = 0.
In my patch, I modified the function to make a table of all flexinode nodes.

4. gone now; forget it. Though really there should be protection in core against anyone trying to use quotes in perms in the future.

5. gone now; forget it.

6. still an issue.

7. still an issue, as far as i can see.
flexinode_field_file_insert calls file_save_upload($node->$fieldname, $node->$fieldname->name)
which will simply save it under the basename of the uploaded file, in the single global directory.
the external path is not qualified by nid.
so if another user uploads a file by the same name, either they will be prevented
by file_save_upload calling file_move with $replace = 0, or they will be allowed,
in which case they overwrite the other user's file (and can make it be deleted later).
My patch qualifies all external destination file names by nid.

jonbob’s picture

jonbob commented

2. Fixed.

3. Will consider as feature request; may be affected by exploitation of the tabbed interface (URLs may change).

7. Calling file_move with $replace = 0 does not deny the file upload; it changes to file name to be unique.

jonbob’s picture

jonbob commented
Version: » master
Anonymous’s picture

(not verified) commented