EAN

please get a review bonus first. Then try to fix issues raised by automated review tools and set this back to "needs review".

Ooops forgot to change the status.

See the automated review here
http://ventral.org/pareview/httpgitdrupalorgsandboxnoe1857888git

Hi Noe_,

The module looks great, but I found some issues :

• There are 3 modules, I think you could move your tpl files, depend on which module use it. You could :
• Move ean-hotel-info.tpl.php in the ean_hotel/ module and create ean_hotel _theme() in this one
• Move ean-search-results.tpl.php in the ean_search/ module and create ean_search_theme() in this one
• Remove the hook_theme() from the common module ean.module.
• For the README.txt. try to choose a style/structure for your readme file. This will improve readability.

Your reviews where pretty short, and please don't put issues back to "needs work" for minor commenting or README.txt style errors. If you find no major problem with a project then RTBC is the right status.

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

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  FILE: /home/klausi/pareview_temp/README.txt
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   49 | ERROR | Files must end in a single new line character
  --------------------------------------------------------------------------------
  
  FILE: /home/klausi/pareview_temp/ean.module
  --------------------------------------------------------------------------------
  FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
  --------------------------------------------------------------------------------
   8 | ERROR | All constants defined by a module must be prefixed with the
     |       | module's name, expected "EAN_OPERATIONS" but found "OPERATIONS"
   9 | ERROR | All constants defined by a module must be prefixed with the
     |       | module's name, expected "EAN_URL" but found "URL"
  --------------------------------------------------------------------------------
  
  FILE: /home/klausi/pareview_temp/ean_hotel/ean_hotel.module
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   102 | ERROR | Files must end in a single new line character
  --------------------------------------------------------------------------------
  

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. project page is too short, see http://drupal.org/node/997024
2. ean_settings_test(): this is vulnerable to XSS exploits. As $form_state['ean_hotel'] stems from a third party source it has to be considered as untrusted user provided input and therefore has to be sanitized before printing. Please read http://drupal.org/node/28984 again. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.
3. ean_hotel_field_formatter_view(): same here: before passing the hotel data to the template you should sanitize all data components.
4. ean_search_search_execute(): same here?
5. ean_http_request(): there is a dsm() debugging left over?

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Sorry for the delay. Make sure to review more project applications and get a new review bonus and this will get finished faster. RTBC = reviewed & tested by the community, this is what we use for applications that look good.

Yes, you need to sanitize stuff from all untrusted sources, except if you already know that it was sanitized at some point before.

1. ean_search_search_execute(): the check_plain() on the $keys is useless since $keys is never used for output? Also, I checked http://api.drupal.org/api/drupal/modules!search!search.pages.inc/functio... and it looks like all properties excpet for $snippet get sanitized anyway. So the only thing you have to do is sanitize the $hotel structure before passing it to ean-search-results.tpl.php and which is later used as $snippet. Either do that in a preprocess function or directly in ean_search_search_execute(). Similar in ean_hotel_field_formatter_view().
2. ean-hotel-info.tpl.php: do not use the ugly str_replace() function here for pseudo security, that should be done before calling the theme() function or in a preprocess hook with wahtever is appropriate http://drupalscout.com/knowledge-base/drupal-text-filtering-cheat-sheet-... .

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

I'm a robot and this is an automated message from Project Applications Scraper.

Updated issue summary.