View Issue Details

IDProjectCategoryView StatusLast Update
0037258LazarusLCLpublic2020-06-28 22:40
Reportereric maw Assigned ToBart Broersma  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformpentium E5700OSWindows 10 home 
Product Version2.0.4 
Fixed in Version2.0.10 
Summary0037258: TStringGrid copy/paste cell containing ampersand enters infinite loop
Descriptionin TCustomStringGrid.SelectionSetHTML, local function ReplaceEntities:

if a cell in a StringGrid contains either:
1) an ampersand, followed later by a semicolon, where the intervening text does not represent a valid HTML entity name.
2) an ampersand, followed later by a character which is converted to an HTML entity, or explicit text representing a valid entity

if the content of such a cell is copied (without invoking an editor instance, so that it is copied to the clipboard as HTML rather than plain text), then pasted into a cell the process does not return.

apparent cause of problem:
-within the processing loop of ReplaceEntities, the scan origin is set to the address of cSt[1] at the start of each loop.
in case 1) a potential entity is found, but is invalid, so that ResolveHTMLEntityReference returns false and the string is not modified. The loop is then restarted at the beginning of the string and the pseudo-entity refound infinitely
in case 2), "&" is correctly replaced by "&", but since the origin is set to the start of the modified string, the inserted "&" is found and initiates a scan which terminates with the end of the following valid entity, but the accumulated substring represents an invalid name and the problem reduces to case 1.
Steps To Reproduce-add a stringgrid component to a form, set goEditing to true, and run the form

-enter either the text "x&x;x" or the text "x&x&x" into a cell
-select the cell (without activating an editor instance) and ctl-C copy
-paste the resultant clipboard contents into a grid cell - the process does not return
Additional InformationI have implemented a simple working modification for my own purposes, but do not do enough coding to feel comfortable with the mix of ansistring, widestring, rawstringbuffer, and (especially) pchar involved, and so not sure that my solution is free of side-effects or is particularly efficient.
TagsNo tags attached.
Fixed in Revisionr63457
LazTarget2.0.10
WidgetsetWin32/Win64
Attached Files

Activities

Bart Broersma

2020-06-27 22:35

developer   ~0123636

Last edited: 2020-06-27 23:55

View 2 revisions

When the cell with 'x&x;x' is copied to the clipboard, the clipboard content should be valid html: 'x&x;x', (i.o.w. the ampersand should be escaped) is this not the case?
[edit]It is correctly escaped upon copy[/edit]

Bart Broersma

2020-06-28 00:41

developer   ~0123643

Last edited: 2020-06-28 00:41

View 2 revisions

Naïve patch attached. Possibly slower than the original which manipulates pchars.
Please test.
grids.SelectionSetHTML.diff (1,350 bytes)   
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 63339)
+++ lcl/grids.pas	(working copy)
@@ -11629,6 +11629,7 @@
   end;
 end;
 
+
 procedure TCustomStringGrid.SelectionSetHTML(TheHTML, TheText: String);
 var
   bStartCol, bStartRow, bCol, bRow: Integer;
@@ -11644,26 +11645,38 @@
     dName: widestring;
     dEntity: WideChar;
   begin
+    result := '';
     while true do begin
-      result := cSt;
       if cSt = '' then
         break;
       o := @cSt[1];
       a := strscan(o, '&');
       if a = nil then
+      begin
+        result := result + cSt;
         break;
+      end;
       b := strscan(a + 1, ';');
       if b = nil then
+      begin
+        result := result + cSt;
         break;
+      end;
       dName := UTF8Decode(copy(cSt, a - o + 2, b - a - 1));
+
+      result := result + copy(cSt, 1, a - o);
+      system.delete(cSt, {a - o + }1, b - a + 2);
+
       dEntity := ' ';
       if ResolveHTMLEntityReference(dName, dEntity) then begin
-        system.delete(cSt, a - o + 1, b - a + 1);
-        system.insert(UTF8Encode(dEntity), cSt, a - o + 1);
+        result := result + dEntity;
+      end
+      else
+      begin
+        result := result + '&' + dName + ';';
       end;
     end;
   end;
-
 begin
   if theHTML <> '' then
   begin
grids.SelectionSetHTML.diff (1,350 bytes)   

Bart Broersma

2020-06-28 15:50

developer   ~0123649

Please test and close if OK.

eric maw

2020-06-28 22:39

reporter   ~0123659

Thanks Bart. This fix works. If this proc is called only during direct user interaction, any speed hit is probably not significant.

Issue History

Date Modified Username Field Change
2020-06-26 01:48 eric maw New Issue
2020-06-27 22:35 Bart Broersma Status new => feedback
2020-06-27 22:35 Bart Broersma LazTarget => -
2020-06-27 22:35 Bart Broersma Note Added: 0123636
2020-06-27 23:54 Bart Broersma Status feedback => confirmed
2020-06-27 23:55 Bart Broersma Note Edited: 0123636 View Revisions
2020-06-28 00:41 Bart Broersma Note Added: 0123643
2020-06-28 00:41 Bart Broersma File Added: grids.SelectionSetHTML.diff
2020-06-28 00:41 Bart Broersma Note Edited: 0123643 View Revisions
2020-06-28 15:46 Bart Broersma Assigned To => Bart Broersma
2020-06-28 15:46 Bart Broersma Status confirmed => assigned
2020-06-28 15:50 Bart Broersma Status assigned => resolved
2020-06-28 15:50 Bart Broersma Resolution open => fixed
2020-06-28 15:50 Bart Broersma Fixed in Version => 2.0.10
2020-06-28 15:50 Bart Broersma Fixed in Revision => r63457
2020-06-28 15:50 Bart Broersma LazTarget - => 2.0.10
2020-06-28 15:50 Bart Broersma Widgetset Win32/Win64 => Win32/Win64
2020-06-28 15:50 Bart Broersma Note Added: 0123649
2020-06-28 22:39 eric maw Note Added: 0123659
2020-06-28 22:40 eric maw Status resolved => closed