Argh! I forgot to put the sample code, sorry... Here it is: https://jsfiddle.net/ym2nuaj2/3/

Able to reproduce the issue and is a regression broken in M50 on Windows 7, MAC and Ubuntu OS. Below are the bisect details for the same.

Good build: 50.0.2658.0
Bad build: 50.0.2660.0

Bisect URL: https://chromium.googlesource.com/chromium/src/+log/c0bf279f397c7d121d0448cf9761a05bbdd2a51a..445b4da887b5413b6c41825af5a3caa3bd706547

Suspecting change #377718 could be the possible suspect.

Review URL: https://codereview.chromium.org/1704103002 .

@jshin: Assigning to you. request you to please take a look into it. Please help us to reassign if not with respect to your change.

Also adding release block label, please undo if not the case.

Thanks.!

@jshin: Gentle ping.

Could you please look into this issue.

Thank you.

A friendly reminder that M50 Stable is launching soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch by Apr-5. All changes MUST be merged into the release branch by 5pm on Apr-8 to make into the desktop Stable final build cut. Thanks!

jshin@: Can we get an update on this issue as it is marked as Stable blocker for M-50. Please review the milestone and the blocker if this is not appropriate.

Cc'ing shess@ (reviewer) for more inputs.

If this will go in the stable channel I will be in serious trouble, please let me know at least if there is some workaround!

M50 Stable is launching very soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged ASAP. All changes MUST be merged into the release branch by 5pm on Apr-8 to make into the desktop Stable final build cut. Thanks!

jshin@, can you pls get this fixed or the culprit CL reverted asap? Thanks.

sorry for missing this bug.

My patch does not deal with an empty string as an input. 

Scott, what's the best way to set the output to an empty string ('zero-length') in sqlite ( in icuCaseFunc16 ) when the input is an empty string (as opposed to null pointer)?   

  zInput = sqlite3_value_text16(apArg[0]);
  if( !zInput ){  //  null is handled, but not an empty string (""). 
    return;
  }

Ok. The upstream patch solved the issue (it does handle an empty input string correctly).  

I lgtm'd it a few weeks ago, but Scott forgot to land. (and, I forgot to send out my draft comment to his another CL. Sorry about that). 

Scott, can you land https://codereview.chromium.org/1746453002/ ? 

If we want to be absolutely safe in M50 branch, an alternative would be to just add a part of the above CL that handles an empty input string? (3 lines of code) to the current tree. 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a8f7233f06c51fcec01dbacf92dfe7c1ec14c25f

commit a8f7233f06c51fcec01dbacf92dfe7c1ec14c25f
Author: jshin <jshin@chromium.org>
Date: Thu Apr 07 02:23:20 2016

[sqlite] Backport icuCaseFunc16 patch from SQLite.

Original Chromium CL at https://codereview.chromium.org/1704103002

"Fix sqlite3's handling of casemapping result 3 times as long as
input"

SQLite interpretation at
http://www.sqlite.org/src/info/b8dc1b9f5d413000

"Fix a potential buffer overflow in the ICU upper() function."

This is a copy of the CL by shess at
   https://codereview.chromium.org/1746453002/ .

BUG= 586079 ,  594478 
TEST=https://jsfiddle.net/ym2nuaj2/3/ (from  bug 594478  comment 1)
TBR=shess@chromium.org

Review URL: https://codereview.chromium.org/1866753005

Cr-Commit-Position: refs/heads/master@{#385621}

[modify] https://crrev.com/a8f7233f06c51fcec01dbacf92dfe7c1ec14c25f/third_party/sqlite/amalgamation/sqlite3.c
[add] https://crrev.com/a8f7233f06c51fcec01dbacf92dfe7c1ec14c25f/third_party/sqlite/patches/0013-backport-Fix-buffer-overrun-in-ICU-extension-s-casem.patch
[delete] https://crrev.com/a37bedae9c23a82bf8b2f9011d06c9933e0048db/third_party/sqlite/patches/0013-icu-Fix-buffer-overflow-when-case-mapping-expands-to.patch
[modify] https://crrev.com/a8f7233f06c51fcec01dbacf92dfe7c1ec14c25f/third_party/sqlite/src/ext/icu/icu.c
[modify] https://crrev.com/a8f7233f06c51fcec01dbacf92dfe7c1ec14c25f/third_party/sqlite/src/test/icu.test

The above CL is not included in the latest Chrome canary.

https://chromium.googlesource.com/chromium/src/+log/51.0.2702.2

We have to wait for one more canary build to bake/verify. 

Verified in 51.0.2703.0 with https://jsfiddle.net/ym2nuaj2/3/

WebSQL is a feature to be deprecated. Anyway, it's a regression so that I guess we want to merge to M50 branch. 

I verified this too, it's working.

I know it's deprecated and we are planning to replace it with indexedDb, but unfortunately it's a big task to do and we can't make it now... So I hope it will continue to work fine as long as it will be still supported by Chrome.

Thank you so much for fixing it anyway!

Sounds like we need a merge request?

Do you want me to handle it?  It _probably_ should go fine.

[Automated comment] Less than a week to go before stable on M50, we might already have a stable candidate build. Manual review required.

Merge approved for M50 (branch 2661). Pls go ahead merge.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6f2e355e6cb7dde3b1e913cd0bc732ba694ef95

commit e6f2e355e6cb7dde3b1e913cd0bc732ba694ef95
Author: Jungshik Shin <jshin@chromium.org>
Date: Tue Apr 12 20:49:36 2016

[sqlite] Backport icuCaseFunc16 patch from SQLite.

Original Chromium CL at https://codereview.chromium.org/1704103002

"Fix sqlite3's handling of casemapping result 3 times as long as
input"

SQLite interpretation at
http://www.sqlite.org/src/info/b8dc1b9f5d413000

"Fix a potential buffer overflow in the ICU upper() function."

The upstream change handles an empty input string while the original
Chromium CL (mine) does not. ( bug 594478 ).

This is a copy of the CL by shess at
   https://codereview.chromium.org/1746453002/ .

BUG= 586079 ,  594478 
TEST=https://jsfiddle.net/ym2nuaj2/3/ (from  bug 594478  comment 1)
TBR=shess@chromium.org

Review URL: https://codereview.chromium.org/1866753005

Cr-Commit-Position: refs/heads/master@{#385621}
(cherry picked from commit a8f7233f06c51fcec01dbacf92dfe7c1ec14c25f)

Review URL: https://codereview.chromium.org/1880263002 .

Cr-Commit-Position: refs/branch-heads/2661@{#571}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/e6f2e355e6cb7dde3b1e913cd0bc732ba694ef95/third_party/sqlite/amalgamation/sqlite3.c
[add] https://crrev.com/e6f2e355e6cb7dde3b1e913cd0bc732ba694ef95/third_party/sqlite/patches/0013-backport-Fix-buffer-overrun-in-ICU-extension-s-casem.patch
[delete] https://crrev.com/12bddafcdd166cdc0471c92b8f079932dfde8a38/third_party/sqlite/patches/0013-icu-Fix-buffer-overflow-when-case-mapping-expands-to.patch
[modify] https://crrev.com/e6f2e355e6cb7dde3b1e913cd0bc732ba694ef95/third_party/sqlite/src/ext/icu/icu.c
[modify] https://crrev.com/e6f2e355e6cb7dde3b1e913cd0bc732ba694ef95/third_party/sqlite/src/test/icu.test

Verified the fix on Windows 7, MAC (10.11.4) & Ubuntu Trusty (14.04) for Google Chrome Beta Version - 50.0.2661.75  

Screen-shot is attached.

TE-Verified labels are added.

Deleted: 594478.png 12.0 KB

	594478.png 12.0 KB View Download