[D7] InstaPost

We 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.

Hi qzmenko,

I found following issues:

1. Should use hook_init() or hook_boot() instead of require_once __DIR__ . '/instapost.forms.inc'.
2. Should use drupal_json_decode() instead of @json_decode.
3. Additional function should use in .inc file like instapost_generate_guid() etc.
4. Should not use "return $form" in instapost_admin_main_settings_submit() in instapost.admin.inc because you already redirecting this form to same page.

Thanks
Rishabh

Hey qzmenko, here's my 2 pennies.
The only reason I've put the module back, is because of the README.txt file (see below). The rest is non-critical.

Automated Review

PASS

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.

No duplication
Yes: Does not cause module duplication and/or fragmentation.

Master Branch
Yes: Follows the guidelines for master branch.

Licensing
Yes: Follows the licensing requirements.

3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template.
At least Introduction, Requirements and Installation sections are mandatory.

Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.

Secure code
Yes: Meets the security requirements.

Coding style & Drupal API usage
- variable_get() used in various places around the module without passing the default value.
- In your instapost_post_to_instagram() there are some hardcoded parameters, like "ig_sig_key_version". Not a big deal, but if anything changes in the API, those keys will have to be patched in the module itself. Also, there are a few other hardcoded elements, especially in the User Agent generation string, which could be parameterised in the future, maybe?
- cookies.txt is stored in the module folder. Consider moving that functionality in the files folder of the site itself (for example), not all webserver setups have write permission set on the modules folder.
- No tests.

- I moved file with cookies in public directory.

I'm not sure that it is a good solution because some hacker can assume that site uses this module and still your Instagram cookie to post some trash into your Instagram account.

I think that this file should be placed on private file system or in temp directory.
By the best way is to get rid of the use of the file.

Hi @qzmenko

instapost.admin.inc

• 19: add variable types in function definition
• 19: also define default values as empty arrays
• 60: add variable types in function definition
• 68: try to avoid of using double quotes, where it unnecessary

instapost.forms.inc

• 53: I think that it will be fine to have default option, like "Select image field"
• 92: why do we need to override default styles?
• 97: do we really need fieldset only for one field?
• 147: try to avoid of using double quotes, where it unnecessary

README.txt

• 16, 19: do not forget to update urls :)

instapost.module

• 31: $output already defined
• 138: instapost_get_node_message contains too many strange operations with message, does it all necessary?
• 215: use drupal_json_encode instead default json_encode
• 244: try to avoid of using double quotes, where it unnecessary
• 246: use drupal_json_encode instead default json_encode
• 251: use REQUEST_TIME constant instead time()
• 332: wrong agrument type, I think it should be string
• 347: wrong agrument type, I think it should be string
• 401: use REQUEST_TIME constant instead time()

Hi qzmenko,

Manual Review:

1. Make use of drupal_http_request() instead of curl_exec() to access external urls.
2. Use password field instead of textfield to get password in admin config page.
3. In system_settings_form() you no need to save form values manually in submit function. It will be automatically saved by default submit function. You can remove the lines 70, 71 from instapost.admin.inc file.
4. As the form is redirecting in to same page, $form_state['redirect'] is not required in instapost.admin.inc line 76.
5. Write hook() functions within instapost.module file instead of in instapost.forms.inc. Also use module_load_include() function to add inc file instead of require_once().

Thanks,
ARUN AK

Closing 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.

Closing 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.

Hello @qzmenko,
I advise you to move .admin.inc and .form.inc to "includes" folder, but it's not blocker for module release.
For me it's ok.

Hello,

I think that this file should be placed on private file system or in temp directory.
By the best way is to get rid of the use of the file.

I think that is a real bad advice, because temp directory is always accessible by any user.

Please check system_menu() function, "system/temporary" item.
Please consider store it in the database or in the private file directory.

Closing 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.

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxKuzmenko2744339git

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

manual review:

1. instapost_generate_guid(): mt_rand() is not suitable for cryptographically secure random data. Use drupal_random_bytes() instead. Not sure if this should be a security blocker, but I guess reversing this GUID is still very hard.
2. instapost_generate_signature(): the third parameter must be a secret key, but you have hard coded a key into the source code for every attacker to see. What is this key? I guess some Instagram private key? Please add a comment in the source code.
3. instapost_admin_main_settings_submit(): so every time the admin form is submitted the variables are overwritten? Shouldn't they be set only once? Please add a comment.

Otherwise looks good to me.

Thanks for your contribution, Anton!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.