Simple Node Importer is a simple module which imports nodes using CSV files. It provides the functionality for mapping UI to map CSV columns to it's corresponding entity field.
Project Page:Simple Node Importer
Git repository: git clone --branch 7.x-1.x https://git.drupal.org/sandbox/sourdrup/2828039.git simple_node_importer
Manual reviews of other projects: on 5th December
https://www.drupal.org/node/2831523#comment-11810553
https://www.drupal.org/node/2829972#comment-11810571
https://www.drupal.org/node/2811601#comment-11810587
Comments
Comment #2
PA robot CreditAttribution: PA robot commentedWe are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)
Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #3
Gomesh CreditAttribution: Gomesh as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedSteps to reproduce the problem while reviewing the module .
1. Select the content type where csv need to be import .
2. then go to simple node import content and add the csv file .
3. Importing the content shows , unable to import few entries . Please go to resultion centre and re-import the file .
4. But after edit the file and added the missing entry , and try to upload the new file .
5. I got the error , I also attached the screenshot .
Comment #4
prasannag CreditAttribution: prasannag as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedI have tried to upload a csv file with the 4 fields/columns(Title, Body, Tags and Image).
Few nodes are missing from import after the batch process.
This is happening when importing taxonomy tags field with multiple values.
Comment #5
Sanjay Chauhan CreditAttribution: Sanjay Chauhan commentedHi @sourdrup,
I'd made a little code review.
simple_node_importer_ajax_breif_note_callback() and simple_node_importer_mapping_form()
- There are Lots of HTML code inside PHP, Kindly use Theme function or create template file.
simple_node_importer_menu()
-The Drupal 6 and 7 menu system stores menu item titles and descriptions in English. This allows the system to cache the data, but display to users using various languages on demand. For this to work, you should not use t() on the title or description of menu items in your hook_menu() implementation. Additionally, you should attempt to use a literal string (rather than a dynamic string) for these two keys, so the translation template extractor can find the string you used.
Regards,
Sanjay Chauhan
Comment #6
rajveergangwarHi
Below are my review:
1) line 252 please use t() for "Kindly contact Admin to select the content ......."
2) Please delete
variable_set('comment_simple_node', COMMENT_NODE_CLOSED);
variable_set('node_options_simple_node', array('status'));
these variable in hook_uninstall
3) Please use t() in you tpl files for
content-info-note.tpl
Mandotory Fields are:
Note: In Multivalue Field List '()' braces contains maximum allowed values.
mapping-help-text-info.tpl
4) please remove patch file.
Comment #7
rajveergangwarchanging status
Comment #8
patilvishalvs CreditAttribution: patilvishalvs as a volunteer commentedManual Review
No duplication
No: There is a Feeds module available, it allows you to map CSV columns with entity fields.
Comment #9
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi patilvishalvs,
Kindly visit the project page of my module Simple Node Importer and see how it is different from the Feeds Module.
Thanks
Comment #10
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi Sanjay Chauhan,
I have fixed all the issues you have mentioned in #5.
Thanks
Comment #11
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi rajveergang,
I have fixed all the issues you have mentioned in #6.
Thanks
Comment #12
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi prasannag,
I have tried to reproduce the error which you mentioned in #4 but fails. I tried 5 files with different scenario but always imported successfully.
Can you please more descriptive or can provide steps to produce.
Thanks
Comment #13
Jabastin Arul CreditAttribution: Jabastin Arul commentedPlease review your code on "https://pareview.sh" site. We found some issue in your code. Here I have attached the link for your error code.
https://pareview.sh/node/162
Comment #14
Jabastin Arul CreditAttribution: Jabastin Arul commentedComment #15
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi Jabastin Arul,
All the errors showing on PAreview.sh has been fixed.
pareview.sh
Thanks for reviewing.
Comment #16
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi @Gomesh,
For #1,
Sorry for late response, actually as per our road map for this, there is no functionality to edit the importer node and can resubmit the new file.
Functionality work flows as:
1. From Resolution Center you can download CSV for failed nodes.
2. Start new import again and upload the file.
I know, on resolution center page under resolution center table, links available for edit, will drive you in confusion, apologies for ignoring that, I have updated the page, now Edit link is no more there.
Thanks for detailing
In case of any issues or suggestions, you're most welcome :)
Comment #17
Ujwala_D CreditAttribution: Ujwala_D as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi @sourdrup,
I have used this module. It seems working fine. PA review issues has been fixed.
Thanks
Comment #18
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedThanks Ujwala_D,
For giving your valuable time in reviewing this module.
Comment #19
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedComment #20
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedComment #21
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedComment #22
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedComment #23
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedComment #24
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedComment #25
sriharsha.uppuluri CreditAttribution: sriharsha.uppuluri at Azri Solutions commentedYou are supposed to add this tag when you review other project applications.
Removing the bonus tag for now. Please add it back once you have reviewed other modules.
Comment #26
sourabh.singhal CreditAttribution: sourabh.singhal as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi @sriharsha.uppuluri ,
Please check the links at the top of issue page,
there are 3 automatic and 3 manual reviews done, all for seperate projects.
Please review again and update us back.
Thanks for the review.
Automatic reviews of other projects: on 1st December
https://www.drupal.org/node/2796471#comment-11805327
https://www.drupal.org/node/2831897#comment-11805329
https://www.drupal.org/node/2831720#comment-11805337
Manual reviews of other projects: on 5th December
https://www.drupal.org/node/2831523#comment-11810553
https://www.drupal.org/node/2829972#comment-11810571
https://www.drupal.org/node/2811601#comment-11810587
Comment #27
prasannag CreditAttribution: prasannag as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi sourdrup,
Thanks for fixing issue mentioned at #4.
No more issues now. Working as mentioned.
Comment #28
nikhilsukul CreditAttribution: nikhilsukul as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commentedHi @sourdrup,
Just reviewed your module. It is working fine for me too.
Comment #29
klausiRemoved automated reviews from the issue summary, not relevant for review bonus.
Comment #30
klausiGit errors:
Review of the 7.x-1.x branch (commit f6e33af):
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:
There are multiple security vulnerabilities in this module and as part of our git admin training I'm assigning this to visabhishek so that he can take a look. If he does not find anything I'm going to post the vulnerability details in one week. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.
Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Comment #31
addonsolutions CreditAttribution: addonsolutions commentedHi visabhishek,
I got this Potential problem on line #258.
use drupal_set_message(check_plain()) instand of drupal_set_message() for security point of view.
Comment #32
addonsolutions CreditAttribution: addonsolutions commentedHi visabhishek,
I got some another issue on this project.
1) Notice: Undefined index: values in simple_node_importer_field_upload_csv_validator()
if user have not selected "Select Content Type" field and trying to upload csv.
2) and change function name taxonomy_term_save instead of taxonomy_save_term on Line #225.
Comment #33
visabhishek CreditAttribution: visabhishek as a volunteer and at Azri Solutions commentedHi addonsolutions,
Thanks for your help, Please allow me some time to finding the security issues.
As you know this is an assignment of git admin training, So I have to find the things as already mentioned by klausi.
Comment #34
visabhishek CreditAttribution: visabhishek as a volunteer and at Azri Solutions commentedHi ,
My findings are :
1: Security vulnerability: On path
simplenode/%/%node/delete
is vulnerable to CSRF exploits. You need to either use a confirmation form or CSRF tokens on GET requests. See http://epiqo.com/de/all-your-pants-are-danger-csrf-explained2: Please use t() for all User facing text
Example :
3: arg(1), arg(2) should be sanitize before printing into #markup.
Example :
4: $record['title'] and $record['body'] should be sanitize
Please let me know if I missed anything.
Comment #35
klausi@visabhishek: yay, great that you found the CSRF issue!
Unfortunately your other points are wrong:
Point 2: you must NOT use t() for watchdog(). Please read the documentation on watchdog().
Point 3: the usage of arg() in l() is not a problem because l() will call check_plain() on the path, so no security issue there. When you suspect a problem in such a case I suggest trying to exploit it, then you quickly realize why it would not work :-)
Point 4: simple_node_importer_make_nodes() must NOT sanitize the data because it does not print anything to HTML. It only saves stuff to the database. Remember: "When handling data, the golden rule is to store exactly what the user typed. When a user edits a post they created earlier, the form should contain the same things as it did when they first submitted it. This means that conversions are performed when content is output, not when saved to the database" from https://www.drupal.org/node/28984 . So no security issue here.
I found simple_node_importer_node_resolution_center(): the $author user name is printed unsanitized to table markup here. Usually that is not a problem because Drupal core does not allow script tags in user names, but third party modules can easily use external user names and we have to always treat them as untrusted user provided data. #theme => 'username' is probably a good solution here.
Comment #36
PA robot CreditAttribution: PA robot commentedClosing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).
I'm a robot and this is an automated message from Project Applications Scraper.