Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check for empty input to change_collation #15031

Merged
merged 2 commits into from Jun 22, 2019

Conversation

sijie123
Copy link
Contributor

@sijie123 sijie123 commented Mar 16, 2019

Description

Currently, if an empty input is provided to change_collation, the behaviour is as if no user input were made.
This causes a bug in db_operations.php, where a blank form will be returned to the user instead of an error message.

By checking for empty($_REQUEST['db_collation']) separately, we can catch bad user input and return the appropriate error message.

Fixes #14987

Before submitting pull request, please review the following checklist:

  • Make sure you have read our CONTRIBUTING.md document.
  • Make sure you are making a pull request against the correct branch. For example, for bug fixes in a released version use the corresponding QA branch and for new features use the master branch. If you have a doubt, you can ask as a comment in the bug report or on the mailing list.
  • Every commit has proper Signed-off-by line as described in our DCO. This ensures that the work you're submitting is your own creation.
  • Every commit has a descriptive commit message.
  • Every commit is needed on its own, if you have just minor fixes to previous commits, you can squash them.
  • Any new functionality is covered by tests. (Not applicable?)

For reviewer:
The github diff is misaligned. I essentially only added lines 86-88 and 136-144

@codecov
Copy link

codecov bot commented Mar 16, 2019

Codecov Report

Merging #15031 into QA_4_8 will decrease coverage by 2.68%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             QA_4_8   #15031      +/-   ##
============================================
- Coverage     55.68%   52.99%   -2.69%     
  Complexity    14452    14452              
============================================
  Files           494      494              
  Lines         70878    64112    -6766     
============================================
- Hits          39467    33976    -5491     
+ Misses        31411    30136    -1275

@codecov
Copy link

codecov bot commented Mar 16, 2019

Codecov Report

❗ No coverage uploaded for pull request base (QA_4_8@d198598). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             QA_4_8   #15031   +/-   ##
=========================================
  Coverage          ?   55.56%           
  Complexity        ?    14498           
=========================================
  Files             ?      494           
  Lines             ?    71071           
  Branches          ?        0           
=========================================
  Hits              ?    39494           
  Misses            ?    31577           
  Partials          ?        0

@williamdes williamdes self-requested a review March 16, 2019 12:30
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sijie123 Your PR works fine, can you implement the same check for tables ?

Currently, if an empty input is provided to change_collation, the behaviour is as if no user input were made.
This causes a bug in db_operations.php, where a blank form will be returned to the user instead of an error message.

By checking for empty($_REQUEST['db_collation']) separately, we can catch bad user input and return the appropriate error message.

Signed-off-by: Si Jie <sijie123@gmail.com>
Fixes: phpmyadmin#14987
Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes williamdes removed this from the 4.8.6 milestone May 28, 2019
@williamdes williamdes added this to Needs triage in issues via automation May 28, 2019
@williamdes williamdes moved this from Needs triage to ready to merge in issues Jun 3, 2019
@williamdes williamdes changed the base branch from QA_4_8 to QA_4_9 June 22, 2019 09:52
@williamdes williamdes added this to the 4.9.1 milestone Jun 22, 2019
@williamdes williamdes merged commit 62d1692 into phpmyadmin:QA_4_9 Jun 22, 2019
issues automation moved this from ready to merge to merged Jun 22, 2019
williamdes added a commit that referenced this pull request Jun 22, 2019
Fixes: #14987
Pull-request: #15031

Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes
Copy link
Member

Thanks for your initial contribution @sijie123 !

@williamdes williamdes added this to Done in pull-requests Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
pull-requests
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants