Javascript cannot read local files for security reasons, so we can't send the data using AJAX. So I have disabled ajax for insert forms which has 'blob' fields. #174

Merged
merged 1 commit into from Feb 21, 2013

Projects

None yet

5 participants

@scnakandala
Contributor

No description provided.

@maduraPradeep

which browser are you used?

@scnakandala
Contributor

which browser are you used?

I am using firefox.

@maduraPradeep

Firefox allow to access local file for firefox extensions. Check mozilla developer documentation.

@scnakandala
Contributor

Hi Madura,
Yes what you said was correct. Using the local link extension for firefox
or the File API in firefox it is possible for web content to access local
files.

On Wed, Feb 20, 2013 at 11:18 AM, madura notifications@github.com wrote:

Firefox allow to access local file for firefox extensions. Check mozilla
developer documentation.


Reply to this email directly or view it on GitHubhttps://github.com/phpmyadmin/phpmyadmin/pull/174#issuecomment-13816984.

Thank you
Supun Nakandala
Dept. Computer Science and Engineering
University of Moratuwa

@scnakandala
Contributor

I also found this link [0] in the web which describes about a browser
independent way to handle ajax file uploads. What is your opinion on how to
solve this error.

[0] - http://www.ajaxf1.com/tutorial/ajax-file-upload-tutorial.html

On Wed, Feb 20, 2013 at 12:00 PM, Supun Nakandala <supun.nakandala@gmail.com

wrote:

Hi Madura,
Yes what you said was correct. Using the local link extension for firefox
or the File API in firefox it is possible for web content to access local
files.

On Wed, Feb 20, 2013 at 11:18 AM, madura notifications@github.com wrote:

Firefox allow to access local file for firefox extensions. Check mozilla
developer documentation.


Reply to this email directly or view it on GitHubhttps://github.com/phpmyadmin/phpmyadmin/pull/174#issuecomment-13816984.

Thank you
Supun Nakandala
Dept. Computer Science and Engineering
University of Moratuwa

Thank you
Supun Nakandala
Dept. Computer Science and Engineering
University of Moratuwa

@zixtor
Contributor
zixtor commented Feb 20, 2013

@scnakandala
While File API and XMLHttpRequest 2 can be used to achieve AJAX file upload and we may use them, but as widely used IE(9 or lower) still lacks support for these HTML 5 capabilities, we also need a fall-back to using older ways. You may also explore uploading files under "import" tab.

@scnakandala
Contributor

@zixtor
I explored the uploading files under "import" tab. It works fine. It is not using ajax handler.

@hisamith hisamith and 1 other commented on an outdated diff Feb 20, 2013
tbl_change.php
@@ -219,7 +219,22 @@
);
//Insert/Edit form
-$html_output .= '<form id="insertForm" method="post" action="tbl_replace.php" name="insertForm" ';
+//If table has blob fields we have to disable ajax.
+$has_blob_field = false;
+for ($i = 0; $i < $columns_cnt; $i++) {
+ $column = $table_fields[$i];
+ if ($column['Type'] == 'blob') {
@hisamith
hisamith Feb 20, 2013 Contributor

Here you are checking only for type 'blob'. So it will fail for other blob types.
I think its better if you use PMA_isColumnBlob($column) function for this check.

@scnakandala
scnakandala Feb 20, 2013 Contributor

Thanks Samith for pointing out that error. I'll correct that.

On Wed, Feb 20, 2013 at 2:19 PM, Samith Dassanayake <
notifications@github.com> wrote:

In tbl_change.php:

@@ -219,7 +219,22 @@
);

//Insert/Edit form
-$html_output .= '<form id="insertForm" method="post" action="tbl_replace.php" name="insertForm" ';
+//If table has blob fields we have to disable ajax.
+$has_blob_field = false;
+for ($i = 0; $i < $columns_cnt; $i++) {

  • $column = $table_fields[$i];
  • if ($column['Type'] == 'blob') {

Here you are checking only for type 'blob'. So it will fail for other blob
types.
I think its better if you use PMA_isColumnBlob($column) function for this
check.


Reply to this email directly or view it on GitHubhttps://github.com/phpmyadmin/phpmyadmin/pull/174/files#r3076809.

Thank you
Supun Nakandala
Dept. Computer Science and Engineering
University of Moratuwa

@ruleant ruleant and 1 other commented on an outdated diff Feb 20, 2013
tbl_change.php
@@ -219,7 +219,22 @@
);
//Insert/Edit form
-$html_output .= '<form id="insertForm" method="post" action="tbl_replace.php" name="insertForm" ';
+//If table has blob fields we have to disable ajax.
+$has_blob_field = false;
+for ($i = 0; $i < $columns_cnt; $i++) {
+ $column = $table_fields[$i];
+ if ($column['Type'] == 'blob') {
+ $has_blob_field = true;
+ break;
+ }
+}
+
+if ($has_blob_field) {
+ $html_output .='<form class="disableAjax" id="insertForm" method="post" action="tbl_replace.php" name="insertForm" ';
@ruleant
ruleant Feb 20, 2013 Contributor
  1. refactor your if statement to only add 'class=...' to the html_output string when there is a blob field. There is no need to repeat all the parameters in both if and else clause.
  2. coding style : wrap long lines, this one is longer than 85 characters
@zixtor
zixtor Feb 20, 2013 Contributor

Also, in the if condition, check for $is_upload to take into account PHP configuration directive.

@zixtor
Contributor
zixtor commented Feb 20, 2013

@scnakandala
Yeah "import" doesn't use AJAX, I meant the emulative submitting to iframe method used in import. Anyways for the purpose of this bug, disabling ajax is fine.

@ruleant ruleant commented on an outdated diff Feb 20, 2013
tbl_change.php
@@ -219,7 +219,20 @@
);
//Insert/Edit form
-$html_output .= '<form id="insertForm" method="post" action="tbl_replace.php" name="insertForm" ';
+//If table has blob fields we have to disable ajax.
+$has_blob_field = false;
+for ($i = 0; $i < $columns_cnt; $i++) {
@ruleant
ruleant Feb 20, 2013 Contributor

You can use a foreach here.

@zixtor
Contributor
zixtor commented Feb 20, 2013

Please combine the commits together and make the commit message meaningful as to indicate which bug it fixes.

@scnakandala
Contributor

@zixtor
Thank you for your advice. Yes I have not used meaningful commit messages. I'll correct that. But can you explain me how can combine the three commits in this pull request.
Thank you.

@ruleant
Contributor
ruleant commented Feb 20, 2013

git reset HEAD~3
will remove the last 3 commits, but the changes to the files will remain. You can then do a new commit with all the changes combined

@zixtor
Contributor
zixtor commented Feb 20, 2013

It seems you have pulled back origin/Branch_blob_error after doing git reset on local copy, doing the following should help :-
git checkout Branch_blob_error
git reset --soft HEAD~5
git commit -am "Fix bug... your new message"
git push -f origin Branch_blob_error

@scnakandala scnakandala This pull request corrects error #3832(http://sourceforge.net/p/phpmy…
…admin/bugs/3832/) by disabling ajax request handler for inserting table rows which has blob fields
6e7aa4f
@ruleant ruleant merged commit e1c82ac into phpmyadmin:master Feb 21, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment