Anonymous File Sharing

You have to post a link to your drupal.org sandbox project for review of application.

i don´t see any readme file in your repository

Nice move, a little more than necessary :) LICENSE.txt is added automatically by drupal.org packaging system, you shouldn't have it in your repo.

Very cool concept and even better it's D7 so I can actually use it. Will review when I find time.

Not using it *yet* -- haven't even looked at the code *yet*, but yeah; subscribing at least.

Saw a nifty and possibly related idea mentioned in IRC -- For image files, make the thumbnail public but the full-sized version private. I can imagine use-cases in online photography sales.

1. Please create and use a branch titled 7.x-1.x rather than the master
2. Remove files[] = anonymous_file_sharing.module and files[] = anonymous_file_sharing.install, that is only for class files. See: http://drupal.org/node/542202
3. Remove $Id tags
4. Functions description comments don't exist, except for basic "implements hook_*".
5. Code generally needs commenting.
6. $node->content['my_additional_field'] = array( - Field needs to be named better
7. 'file_validate_extensions' => array('txt jpg png gif doc rtf xls csv wav mp3 wma mpg avi bmp'), - zip? gz? jpeg? rar?
8. "File Key" needs a better description or name.

Review of the 7.x-1.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/anonymous_file_sharing.module:
   +308: [minor] There should be no trailing spaces
   +356: [minor] There should be no trailing spaces
   +363: [minor] There should be no trailing spaces
   +378: [minor] There should be no trailing spaces
   +502: [minor] Comment should be read "Implements hook_foo()."
  
  sites/all/modules/pareview_temp/test_candidate/anonymous_file_sharing.install:
   +36: [minor] There should be no trailing spaces
   +37: [minor] There should be no trailing spaces
  
  Status Messages:
   Coder found 1 projects, 2 files, 7 minor warnings, 0 warnings were flagged to be ignored
  

• Lines in README.txt should not exceed 80 characters, see the guidelines for in-project documentation.
• Comments: there should be a space after "//", see http://drupal.org/node/1354#inline
  

  anonymous_file_sharing.module:52:  //First : List all "anonymous files" nodes to delete.
  anonymous_file_sharing.module:59:    //Secondly : Delete all the nodes (attached files will be destroyed too).
  anonymous_file_sharing.module:62:    //Finally : Delete the id of the node destroyed in deletion queue.
  

  Also in many other places.

• ./anonymous_file_sharing.module: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
  

    //If the node identifier is in deletion queue, this mark will be erased till new limits for file are reached.
      //... and this time limit is reached, then we unactivate download and destroy it, if not reached we will consider next conditions.
      //If file is not marked for deletion queue but if it will be destroyed because of its conditions are reached, we pull it in deletion queue.
      //If download is not forbidden, we give an URL to next step of download process... else, we deny access to this file.
  

  Also in many other places.

• Do not use t() in hook_schema(), this will only generate overhead for translators.
  

      'description' => $t('The base table for anonymous file deletion queue.'),
          'description' => $t('The node that has anonymous file to delete.'),
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Hi,

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

Review of the 7.x-1.x branch:

• Lines in README.txt should not exceed 80 characters, see the guidelines for in-project documentation.
• Comments: there should be a space after "//", see http://drupal.org/node/1354#inline
  

  anonymous_file_sharing.module:55:  //First : List all "anonymous files" nodes to delete.
  anonymous_file_sharing.module:62:    //Secondly : Delete all the nodes (attached files will be destroyed too).
  anonymous_file_sharing.module:65:    //Finally : Delete the id of the node destroyed in deletion queue.
  anonymous_file_sharing.module:131:  //Just for "anonymous files" nodes :
  anonymous_file_sharing.module:133:    //If the file has a condition of time limit...
  anonymous_file_sharing.module:405:  //Preparing HTTP headers for sending file...
  anonymous_file_sharing.module:421:  //Sending file.
  anonymous_file_sharing.module:540:  //We generate a random title used for path thanks to Token module or not.
  anonymous_file_sharing.module:559:  //If the file is "anonymous", there is no need to display it on front page...
  anonymous_file_sharing.module:562:  //Save the new anonymous file as a node of content type "anonymous file".
  anonymous_file_sharing.module:565:  //then, give the URL to share for downloads.
  

• There should be no space after the opening "(" of an array, see http://drupal.org/node/318#array
  

  anonymous_file_sharing.install:145:        'allowed_values' =>  array( "60" => "One Minute",
  

• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./anonymous_file_sharing.info ./README.txt ./anonymous_file_sharing.module ./anonymous_file_sharing.install
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

HTH, dave

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

Review of the 7.x-1.x branch:

• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Manual review (in brief):

• I have checked out your files as a snapshot. And also as raw data directly from the repo. Actually I receive two newlines at the file end. There is indeed something weird, but I did not yet figure out what it is so, as for me, this would not block.
• There are still files in your master branch. You should replace them as described in http://drupal.org/node/1127732 at step 5.
• Check out the tips for a great project page. Not just for my better understanding as I would go into deeper tests next, but also for a clearer description for anyone. Although I have a clue which problem you want to solve, I still have to guess a bit. So just try to illustrate the whole thing a bit (with words and, eventually, a screen shot or two)?
• Also, please make some statements on similar existing projects and, if there are any, on the differences between them and your project.

HTH, dave

Hi,

I can't add screenshots on Edit project page

That's partly correct, but you may host them outside and just reference them with a link atm. :) Putting them in the repo is no problem IMO, but not very helpful for a site builder's first glance (which often decides about the downloading and trial of a module). But I would not recommend putting them in the master branch (which, as a result, is not empty again then ;))

Ok:

• You should add to your docs (readme, project page) that this is for D7 only (yet). I missed that out at first.
• Now some really advanced nit-picking ;) You are using selects instead of checkboxes (easier, more logical, less space-consuming, less code for FAPI) in your form block (module file, line 484). Are you aware of the interface standards and related docs with their recommendations? Though I would not consider this as a blocking issue, perhaps you might want to check that again?

BTW: I subscribe that this is a nice project. Will you backport it to D6 as well? :)

@nicolaspalmier has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

Hi, you marked as active (which I think is not used for applications, or?), but you say you will work on this later, could we mark this as postponed until you have the time to work on it?

When I try to install the module I get two errors:
- first it complaints if private file system is not set: if this is a requirement, you should check this in hook_requirements() beforehand and act appropriately.
- WD php: Recoverable fatal error: Argument 2 passed to t() must be an array, string given, called in ...anonymous_file_sharing.install on line 107

Aside the mentioned problems the code looks quite solid and clean. Nice job!

Hi Nicolas, thanks for your work. Cool dropbox-style file uploads by unauthenticated users. Unlike the commercial Dropbox, but like a classic spy "dropbox."

• First, let me say that I think this module invites some degree of abuse, because uploaded files are commonly exploited to deliver malware or various forms of browser abuse. It's true that it's only a tool and people could use it for wholesome purposes as well, but in my opinion you should warn users about the dangers of installing a module that allows unauthenticated users to post pretty much any file they want, since it's easy to get around file extension restrictions. Where exactly to warn? I'm not sure. Perhaps in the README as a start.
• The term "anonymous" means something quite different for us Drupal folks, versus the general public. I know what you mean, regarding the fact that users do not need to be "authenticated" to share files, but I think calling this "anonymous" filesharing is still a bit misleading. How are users' identities actually protected or made anonymous? If there's no good answer for this, perhaps you should consider calling this "unprivileged" or "unauthenticated" or "common," or something similar. It's not too late at this point, though I'm well aware it would be a bit of a pain.
• Please consider adding this to your README, or at least refer, somewhere, to http://drupal.org/documentation/modules/file:
  

  PHP configuration

  For file uploads to work, PHP must be configured properly. The following PHP configuration variables may need to be set or configured, in your PHP php.ini file, .htaccess file, or settings.php files.

  file_uploads = On must be set to "On"
  upload_max_filesize = 24M can't be larger than post_max_size
  max_input_time = 300 small values may cause timeouts for large file uploads
  memory_limit = 64M small values may cause out of memory errors for large file uploads
  max_execution_time = 180 small values may cause timeouts for large file uploads
  post_max_size = 24M limits the size of input submitted to the website (including attached files)

• The online review system found some code issues, as you may expect. If you visit http://ventral.org/pareview/httpgitdrupalorgsandboxnicolaspalmier1248312... , you can see these for yourself. Plenty to look at, since they're continually updating the script.

All in all, though, an interesting idea, and I look forward to it.

keep it in needs work until issues are resolved.

I love "Drupbox" for a name.

It is a really cool name.

I'm a little concerned for you about the similarity to the Dropbox name, since their Terms of Service indicate that "These Terms do not grant you any rights to use the Dropbox trademarks, logos, domain names, or other brand features," and this may produce "confusion in the mind of the consumer." Since the venue for any legal disagreement is probably going to be California, you may want to check with a lawyer there who specializes in copyright and intellectual property.

Be careful about what you tell people on your project page, some of them won't know what you mean with "A check is made during hook_requirements step". Don't get too technical! Tell them that they can find it at the status report page.
Also nobody (I know) reads readme's try to put all important information onto the project page.

description = Shares files anonymously Maybe rather "Provides a system for sharing files anonymously" ? that kind of pattern would follow most other module descriptions

Don't make the descriptions of in your schema's translatable! (hook_menu(), hook_schema() and watchdog() are the places where you should never use such stuff).
Also drupal already has a nice cron-queue api / general queue api, you should have a look on these before reinventing the wheel here.

all - your variables must be prefixed with the full module name!

install, line 41: $types = node_type_get_types(); - what for?

It's not common to use underscores in permission name strings (anyway you can call them however you like)

I'd suggest you to use $items['afs/%node'] for your path, this way it's required that this path get's a node id instead of the title, however it's much easier to handle as you directly get the node object by the parameter and the existance of the node is checked automatically and you don't have to care about this.
To keep it working with nodetitle's in url's you can simply make use of path aliases (this would be the same like node and autopath works, nodes only care about node/%node and autopath give's the possibility to use any other path instead)

sha1(rand()); commonly here's drupal_hash_base64(drupal_random_bytes(55)); used instead

'#value' => 'Download', make all human readable stuff translatable with t() (sure with the exceptions mentioned above)

your always doing a space between isset and (, stop it! ;P

'TRUE' => t('Yes'),, '60' => t('One Minute'), Got a boolean? use a it as boolean not as string! Got a number? use it as number not as string! - don't make php suffer of instant variable casting ;)

Please resolve at least all major (bold) issues, the other's are personal suggestions (I highly recommend :P)

regards

Nicolas, thank you for your patience! We know that the review process can be somewhat lengthy. Please review the "Application Review Timelines" (scroll down that page) in order to adjust the priority of this project application as appropriate.

In addition, have you already discovered that you can get your issue advanced in the review queue (a "review bonus"), if you review other project applications?

"sharemyfile" and "sharedfile" seem fine to me.

Don't RTBC your own issues. The application review timelines only state that can increase the priority of your issue after a certain timeframe.

Hi, - I have the following suggestions to you for the code.
in anonymous_file_sharing.module:
1. use underscore ("_") for function names you intend for module's use only.

2.
line 74
function anonymous_file_sharing_node_presave($node) {
--- Indicate in comments that this is a hook implementation

3.
line 105
function anonymous_file_sharing_node_view($node, $view_mode, $langcode) {
--- Indicate in comments that this is a hook implementation

4.
line 286
function anonymous_file_sharing_download_form_validate($form, &$form_state) {
--- Indicate in comments that this is a hook implementation

5.
line 328
function anonymous_file_sharing_download_form_submit($form, &$form_state) {
--- Indicate in comments that this is a hook implementation

6.
line 328
function anonymous_file_sharing_download_form_submit($form, &$form_state) {
--- Is this a hook?

7.
line 445
function anonymous_file_sharing_block_view($delta = '') {
--- Indicate in comments that this is a hook implementation

8.
line 460
function anonymous_file_sharing_create_form($form, &$form_state) {
--- Is this a hook?

9.
line 524
function anonymous_file_sharing_create_form_submit($form, &$form_state) {
--- Indicate in comments that this is a hook implementation

Optional:
1. For text strings use either ' or " consistently. Some of your t() use ", while other '.
2. For expressions like this one
338 $file_one_time = $temp_node->afs_one_time[LANGUAGE_NONE][0]['value'];
you may want to assign the value to a variable and reuse this variable in the code rather than doing the
evaluation several times. This will make the code more readable, too.

I think this should be marked as needs work after #43.

Bonjour Nicolas,

1. First thing I would suggest to get your module approved is to follow the guidelines for the approval process:
http://drupal.org/node/1011698

• Ok - A detailed description of what your project does, including how it is different from other, similar projects, if applicable.
• Missing - A link to your project page.
• Missing - A direct link to your git repository (git clone ...).
• Missing - Please specify if it's for Drupal 6 and/or 7.
• Not mandatory but strongly adviced A list of links to reviews of other project applications that you did.

Those informations should be added in the issue description, not inside a comment. That will make the reviewers task easier and you will get more chances of getting the full project permission.

2. You should remove Master branch and check that you have set the default branch.
see: http://drupal.org/node/1659588 and http://drupal.org/node/1127732

3. There are two errors regarding the code standards:

FILE: ...all/modules/pareview_temp/test_candidate/anonymous_file_sharing.install
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
139 | ERROR | Function comment short description must be on a single line
173 | ERROR | Expected 1 space after "=>"; 2 found
--------------------------------------------------------------------------------

See: http://ventral.org/pareview/httpgitdrupalorgsandboxnicolaspalmier1248312git

4. You set variables on hook_install, this is not needed as you can use the default value in the variable_get function.

5. This is not really a bug but more a suggestion. The allowed extensions could be configured dynamically on a config page instead hard coding them on the module installation. Beside that there should be also free extensions by default like the ogg format for video, and not only privative extensions as avi or mp3. Well maybe it should be considered as a bug :-)

6. This is very important ! There is a problem on line 157 of the install file. You have not set properly the pair 'value' => 'label', then you create a form again on the module file (line 477) that is never called. Instead of that you should call hook_form_FORM_ID_alter to alter the already existing creation form for the node. You can read again the hook definition to get the details api.drupal.org/api/drupal/modules!system!system.api.php/function/hook_form_FORM_ID_alter/7

Please keep the good job correcting the issues as I found this module very interesting and I'm sure lot of people can get a good use of this.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.