View Issue Details

IDProjectCategoryView StatusLast Update
0031756Open CASCADEOCCT:Data Exchangepublic2020-12-29 12:55
Reporterabv Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Target Version7.6.0Fixed in Version7.6.0 
Summary0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
DescriptionThe problem has been discovered during analysis of regression manifested on test case bugs step bug26451 with a fix made for #31489.

The STEP entity #71919 (and many others) in the file attached to issue 0026451 has last parameter missing, like this:

#71919=EDGE_CURVE('',#84041,#84041,#0, );
#71920=FILL_AREA_STYLE('',(#84042));

When this STEP file is read, such entities are parsed incorrectly: instead of incomplete list of parameters, they get extra parameters by including ones from the next entity, and that next entity gets ignored:

~~~~~
Draw[1]> test bugs step bug26451
...
Draw[2]> entity #71919
  -- DUMP Entity n0 71355 level 1
 --- (STEP) Entity 71355:#71919 Type cdl : StepShape_EdgeCurve
 *** NOT WELL LOADED : CONTENT FROM FILE ***
In dump, iii:#jjj means : entity rank iii has step ident #jjj
71355:#71919 = EDGE_CURVE('',83437:#84041,83437:#84041,#0,#0,/* (SUB) */
  (83438:#84042));
  /* On Entity above, Fail Messages recorded at Read time :
Count of Parameters is not 5 for edge_curve
A reference to another entity is unresolved */
 ** Data Check (One Entity) **
Check:1 -- Entity (n0:id) 71355:#71919 Type:EDGE_CURVE
Count of Parameters is not 5 for edge_curve
A reference to another entity is unresolved

Draw[3]> entity #71920
Error in Command : entity #71920
~~~~~

I have prepared synthetic test file for this case, attached as bug31756.stp.
Desired behavior is that the file should be read successfully (it is just a sphere) but error shall be generated on entity 0000015.

Current behavior is:
- on master branch: translation fails, with error on entity 0000015 (wrong nb. of parameters)
- on CR31489_1: translation succeeds without any messages

Note that while work-around implemented in CR31489_1 effectively works in this particular case, this issue shows that our STEP parser can be broken easily by incorrect data, and its behavior becomes unpredictable. We need to understand how original problem occurs and prevent this in more generic way.
Steps To Reproducepload ALL
stepread [locate_data_file bug31756.stp] a *
TagsNo tags attached.
Test case numberNot required

Attached Files

  • bug31756.stp (2,175 bytes)

Relationships

related to 0026451 closedbugmaster Community Crash importing STeP file 
parent of 0032028 closedbugmaster Open CASCADE Coding Rules, StepFile - eliminate CLang warning -Wmisleading-indentation 

Activities

abv

2020-09-07 14:39

manager  

bug31756.stp (2,175 bytes)

git

2020-11-30 11:42

administrator   ~0097155

Branch CR31756_testParser has been created by dpasukhi.

SHA-1: 225b5fa264dcbc01d8edfe2e4c56775917347288


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    Recorder made thread safe (converted from c to c ++)
    Delete useless files ( stepread.cxx, stepread.ph )

git

2020-12-03 02:00

administrator   ~0097255

Branch CR31756_1 has been created by dpasukhi.

SHA-1: 8c27092e8209ceeeb6a97a5e33a19db099619de0


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Recorder may safely be used in a multi-threaded environment.
     - Transform "C" recfile to "C++" RecordFile
    - Fix problem with missing last parameters
    - Rename files and methods of recfile ( RecordFile )
    - Delete useless files ( stepread.cxx, stepread.ph )

git

2020-12-04 15:07

administrator   ~0097323

Branch CR31756_2 has been created by dpasukhi.

SHA-1: 67ab474f6b69bf2882d1e30b9984d13307ddd9f6


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Recorder may safely be used in a multi-threaded environment.
     - Transform "C" recfile to "C++" RecordFile
    - Fix problem with missing last parameters
    - Rename files and methods of recfile ( RecordFile )
    - Delete useless files ( stepread.cxx, stepread.ph )
    - Add control of arguments count ( special for error argument )

Author: dpasukhi
Date: Tue Nov 24 15:52:39 2020 +0300

    0031000: Data Exchange - STEP file cannot be opened due to lower-case special token End-ISO-10303-21
    
    Update flex lexical scanner (step.lex lex.step.cxx) to parse special tokens in a case insensitive manner

dpasukhi

2020-12-04 17:18

developer   ~0097328

Dear ABV,
Please review CR31756_2.
This branch contains two resolved issue: current and 0031000 ( touch same files )

Current branch fix undefined behavior of parser ( skip or rewrite entity )

RecordFile.cxx:71 contains non-ASCI symbol ('C'), I'll fix it with rebase to new master or with remarks

Rebase "recfile" from "C" to "C++" don't have regressions for time or memory.

All tests are OK, no regressions, see:
http://occt-tests/CR31756-master-dpasukhi-OCCT/Debian80-64/diff_summary.html
http://occt-tests/CR31756-master-dpasukhi-OCCT/Windows-64-VC14/diff_summary.html
http://occt-tests/CR31756-master-dpasukhi-Products/Debian80-64/diff_summary.html
http://occt-tests/CR31756-master-dpasukhi-Products/Windows-64-VC14/diff_summary.html

abv

2020-12-06 18:25

manager   ~0097398

Please consider some remarks:

- New class "RecordFile" should be named according to OCCT coding conventions, with package prefix "StepFile_". I would suggest name "StepFile_Model" or "StepFile_DataModel" or similar, reflecting the fact that this class defines data structures representing the contents of the STEP file (if my understanding is correct indeed).
- It would be nice to add some description of this class (logical: purpose, way of using, including any relevant notes on use in multithreaded contexts) as the class documentation.
- In StepFile_Read, instance of RecordFile object can be made a local variable to avoid need of error-prone new/delete actions
- Method RecordDebList() can be better named RecordListStart() to exclude use of legacy French terms (deb = debut I guess)

git

2020-12-08 18:08

administrator   ~0097428

Branch CR31756_3 has been created by dpasukhi.

SHA-1: 1c902294bab7f96b9c7b4096e0f64baee19c0818


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Recorder may safely be used in a multi-threaded environment.
     - Transform "C" recfile to "C++" StepFile_DataModel
    - Fix problem with missing last parameters
    - Rename files and methods of recfile ( StepFile_DataModel )
    - Delete useless files ( stepread.cxx, stepread.ph )
    - Add control of arguments count ( special for error argument )
    - Add description of StepFile_DataModel class

Author: dpasukhi
Date: Tue Nov 24 15:52:39 2020 +0300

    0031000: Data Exchange - STEP file cannot be opened due to lower-case special token End-ISO-10303-21
    
    Update flex lexical scanner (step.lex lex.step.cxx) to parse special tokens in a case insensitive manner

git

2020-12-09 00:01

administrator   ~0097434

Branch CR31756_3 has been updated forcibly by dpasukhi.

SHA-1: 734a0192983bda65a3e304397b753a24ba43cbf5

git

2020-12-09 11:20

administrator   ~0097435

Branch CR31756_3 has been updated forcibly by dpasukhi.

SHA-1: 9d531d0fb491e8b5f8e8806bf2f11e7b501e9729

dpasukhi

2020-12-09 11:34

developer   ~0097436

Dear ABV,
All remarks have been done.
Please review CR31756_3.

All test are OK, see,
- restart:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31756-master-dpasukhi/view/TESTING/job/CR31756-master-dpasukhi-OCCT-Windows-64-VC14-opt-test-restart/HTML_20Report/
- tests:
http://occt-tests/CR31756-master-dpasukhi-OCCT/Debian80-64/diff_summary.html
http://occt-tests/CR31756-master-dpasukhi-OCCT/Windows-64-VC14/diff_summary.html
http://occt-tests/CR31756-master-dpasukhi-Products/Debian80-64/diff_summary.html
http://occt-tests/CR31756-master-dpasukhi-Products/Windows-64-VC14/diff_summary.html

git

2020-12-11 18:57

administrator   ~0097507

Branch CR31756_3 has been updated by abv.

SHA-1: b0ad5789e7988a63f65fc66684d36e1c26d43dfa


Detailed log of new commits:

Author: abv
Date: Fri Dec 11 18:50:43 2020 +0300

    // StepFile_DataModel renamed to StepFile_ReadData

git

2020-12-11 19:20

administrator   ~0097510

Branch CR31756_3 has been updated forcibly by abv.

SHA-1: a022b5e458ece2597d213fee763e3ad1d3c88ed3

git

2020-12-11 22:21

administrator   ~0097512

Branch CR31756_4 has been created by abv.

SHA-1: a524d2359be3ede0b4807b09739cffa3c7548e0b


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Parser is corrected to handle case of missing arguments properly (report error without corruption of the next entity)
    - Added counter of entity arguments for appropriate error messages
    - Plain C tools and data structures (recfile.*, stepfile.*) are converted to C++ class (StepFile_ReadData) to avoid use of static variables, so that reader can be safely used in a multi-threaded environment

abv

2020-12-12 07:44

manager   ~0097515

Reviewed with minor amendments (new class renamed) and tested, see Jenkins job CR31756-master-dpasukhi.
Please integrate:
OCCT branch CR31756_4
Products: none

git

2020-12-16 11:53

administrator   ~0097628

Branch CR31756_5 has been created by dpasukhi.

SHA-1: 726f0fe206884aeb0950cd278f86babdbda6b1fc


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Parser is corrected to handle case of missing arguments properly (report error without corruption of the next entity)
    - Added counter of entity arguments for appropriate error messages
    - Plain C tools and data structures (recfile.*, stepfile.*) are converted to C++ class (StepFile_ReadData) to avoid use of static variables, so that reader can be safely used in a multi-threaded environment

git

2020-12-16 12:05

administrator   ~0097631

Branch CR31756_4_1 has been created by dpasukhi.

SHA-1: 271f53ddd82e3e2b6a5c0c0ee9fcae865a3d8c0e


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" warning

git

2020-12-17 11:00

administrator   ~0097656

Branch CR31756_4_2 has been created by dpasukhi.

SHA-1: 128182ac39bf781c4822f4621fa02964e105e163


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" and "-Wunknown-pragmas" warnings

git

2020-12-17 12:47

administrator   ~0097672

Branch CR31756_4_2 has been updated forcibly by dpasukhi.

SHA-1: 1bb778940b1d8a521c964b18518716898f0890ef

git

2020-12-18 12:32

administrator   ~0097702

Branch CR31756_4_3 has been created by dpasukhi.

SHA-1: b1019f95e9f602a6764b0ccb869871c8dac9a821


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" and "-Wunknown-pragmas" warnings

git

2020-12-18 14:32

administrator   ~0097711

Branch CR31756_4_4 has been created by dpasukhi.

SHA-1: b5769181b2000bc1058e681918840a519cf22c51


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" warning

git

2020-12-21 14:10

administrator   ~0097829

Branch CR31756_4_4 has been updated forcibly by dpasukhi.

SHA-1: 1fa1ebcfa3b2b1e530eb1fd843e2377cb0f36b50

bugmaster

2020-12-26 12:15

administrator   ~0097916

Combination -
OCCT branch : WEEK-52
master SHA - 41046145c4a15f5cedf5f3c5877952ee00d568b4
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : WEEK-52 SHA - 290e5c74e8fef71947cadf90acb8e43c81ed10a1
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17722.770000000193 / 17710.950000000055 [+0.07%]
Products
Total CPU difference: 12416.490000000109 / 12412.520000000126 [+0.03%]
Windows-64-VC14:
OCCT
Total CPU difference: 19321.5625 / 19274.6875 [+0.24%]
Products
Total CPU difference: 13844.421875 / 13853.046875 [-0.06%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2020-12-26 14:52

administrator   ~0097936

Branch CR31756_1 has been deleted by inv.

SHA-1: 8c27092e8209ceeeb6a97a5e33a19db099619de0

git

2020-12-26 14:53

administrator   ~0097937

Branch CR31756_2 has been deleted by inv.

SHA-1: 67ab474f6b69bf2882d1e30b9984d13307ddd9f6

git

2020-12-26 14:53

administrator   ~0097938

Branch CR31756_3 has been deleted by inv.

SHA-1: a022b5e458ece2597d213fee763e3ad1d3c88ed3

git

2020-12-26 14:53

administrator   ~0097939

Branch CR31756_4 has been deleted by inv.

SHA-1: a524d2359be3ede0b4807b09739cffa3c7548e0b

git

2020-12-26 14:53

administrator   ~0097940

Branch CR31756_4_1 has been deleted by inv.

SHA-1: 271f53ddd82e3e2b6a5c0c0ee9fcae865a3d8c0e

git

2020-12-26 14:53

administrator   ~0097941

Branch CR31756_4_2 has been deleted by inv.

SHA-1: 1bb778940b1d8a521c964b18518716898f0890ef

git

2020-12-26 14:53

administrator   ~0097942

Branch CR31756_4_3 has been deleted by inv.

SHA-1: b1019f95e9f602a6764b0ccb869871c8dac9a821

git

2020-12-26 14:53

administrator   ~0097943

Branch CR31756_4_4 has been deleted by inv.

SHA-1: 1fa1ebcfa3b2b1e530eb1fd843e2377cb0f36b50

git

2020-12-26 14:53

administrator   ~0097944

Branch CR31756_5 has been deleted by inv.

SHA-1: 726f0fe206884aeb0950cd278f86babdbda6b1fc

git

2020-12-26 14:53

administrator   ~0097945

Branch CR31756_testParser has been deleted by inv.

SHA-1: 225b5fa264dcbc01d8edfe2e4c56775917347288

Related Changesets

occt: master 2057c37b

2020-11-27 19:32:18

dpasukhi


Committer: bugmaster Details Diff
0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter

- Parser is corrected to handle case of missing arguments properly (report error without corruption of the next entity)
- Added counter of entity arguments for appropriate error messages
- Plain C tools and data structures (recfile.*, stepfile.*) are converted to C++ class (StepFile_ReadData) to avoid use of static variables, so that reader can be safely used in a multi-threaded environment
Affected Issues
0031756
mod - src/StepFile/FILES Diff File
mod - src/StepFile/lex.step.cxx Diff File
rm - src/StepFile/recfile.pc Diff File
rm - src/StepFile/recfile.ph Diff File
mod - src/StepFile/step.lex Diff File
mod - src/StepFile/step.tab.cxx Diff File
mod - src/StepFile/step.tab.hxx Diff File
mod - src/StepFile/step.yacc Diff File
mod - src/StepFile/StepFile_Read.cxx Diff File
add - src/StepFile/StepFile_ReadData.cxx Diff File
add - src/StepFile/StepFile_ReadData.hxx Diff File
rm - src/StepFile/stepread.cxx Diff File
rm - src/StepFile/stepread.ph Diff File
add - tests/bugs/step/bug31756 Diff File

Issue History

Date Modified Username Field Change
2020-09-07 13:20 abv New Issue
2020-09-07 13:20 abv Assigned To => dpasukhi
2020-09-07 13:20 abv Relationship added related to 0026451
2020-09-07 14:39 abv File Added: bug31756.stp
2020-09-07 14:45 abv Description Updated
2020-09-07 14:45 abv Steps to Reproduce Updated
2020-09-17 19:49 gka Status new => assigned
2020-09-22 18:28 szy Target Version 7.5.0 => 7.6.0
2020-11-30 11:42 git Note Added: 0097155
2020-12-03 02:00 git Note Added: 0097255
2020-12-04 15:07 git Note Added: 0097323
2020-12-04 17:18 dpasukhi Note Added: 0097328
2020-12-04 17:18 dpasukhi Assigned To dpasukhi => abv
2020-12-04 17:18 dpasukhi Status assigned => resolved
2020-12-06 18:25 abv Note Added: 0097398
2020-12-06 18:25 abv Assigned To abv => dpasukhi
2020-12-06 18:25 abv Status resolved => assigned
2020-12-08 18:08 git Note Added: 0097428
2020-12-09 00:01 git Note Added: 0097434
2020-12-09 11:20 git Note Added: 0097435
2020-12-09 11:34 dpasukhi Note Added: 0097436
2020-12-09 11:34 dpasukhi Assigned To dpasukhi => abv
2020-12-09 11:34 dpasukhi Status assigned => resolved
2020-12-11 18:57 git Note Added: 0097507
2020-12-11 19:20 git Note Added: 0097510
2020-12-11 22:21 git Note Added: 0097512
2020-12-12 07:44 abv Note Added: 0097515
2020-12-12 07:44 abv Assigned To abv => bugmaster
2020-12-12 07:44 abv Status resolved => reviewed
2020-12-16 11:53 git Note Added: 0097628
2020-12-16 12:05 git Note Added: 0097631
2020-12-17 11:00 git Note Added: 0097656
2020-12-17 12:47 git Note Added: 0097672
2020-12-18 12:32 git Note Added: 0097702
2020-12-18 14:32 git Note Added: 0097711
2020-12-18 19:26 bugmaster Assigned To bugmaster => dpasukhi
2020-12-18 19:26 bugmaster Status reviewed => feedback
2020-12-21 14:10 git Note Added: 0097829
2020-12-23 19:32 bugmaster Assigned To dpasukhi => bugmaster
2020-12-23 19:32 bugmaster Status feedback => reviewed
2020-12-26 12:15 bugmaster Note Added: 0097916
2020-12-26 12:15 bugmaster Status reviewed => tested
2020-12-26 12:21 bugmaster Test case number => Not required
2020-12-26 14:40 bugmaster Changeset attached => occt master 2057c37b
2020-12-26 14:40 bugmaster Status tested => verified
2020-12-26 14:40 bugmaster Resolution open => fixed
2020-12-26 14:52 git Note Added: 0097936
2020-12-26 14:53 git Note Added: 0097937
2020-12-26 14:53 git Note Added: 0097938
2020-12-26 14:53 git Note Added: 0097939
2020-12-26 14:53 git Note Added: 0097940
2020-12-26 14:53 git Note Added: 0097941
2020-12-26 14:53 git Note Added: 0097942
2020-12-26 14:53 git Note Added: 0097943
2020-12-26 14:53 git Note Added: 0097944
2020-12-26 14:53 git Note Added: 0097945
2020-12-29 12:55 kgv Relationship added parent of 0032028