View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0031756 | Open CASCADE | OCCT:Data Exchange | public | 2020-09-07 13:20 | 2020-12-29 12:55 |
Reporter | Assigned To | bugmaster | |||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Target Version | 7.6.0 | Fixed in Version | 7.6.0 | ||
Summary | 0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter | ||||
Description | The 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 Reproduce | pload ALL stepread [locate_data_file bug31756.stp] a * | ||||
Tags | No tags attached. | ||||
Test case number | Not required | ||||
2020-09-07 14:39 manager |
bug31756.stp (2,175 bytes) |
|
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 ) |
|
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 ) |
|
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 |
|
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 |
|
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) |
|
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 |
|
Branch CR31756_3 has been updated forcibly by dpasukhi. SHA-1: 734a0192983bda65a3e304397b753a24ba43cbf5 |
|
Branch CR31756_3 has been updated forcibly by dpasukhi. SHA-1: 9d531d0fb491e8b5f8e8806bf2f11e7b501e9729 |
|
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 |
|
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 |
|
Branch CR31756_3 has been updated forcibly by abv. SHA-1: a022b5e458ece2597d213fee763e3ad1d3c88ed3 |
|
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 |
|
Reviewed with minor amendments (new class renamed) and tested, see Jenkins job CR31756-master-dpasukhi. Please integrate: OCCT branch CR31756_4 Products: none |
|
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 |
|
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 |
|
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 |
|
Branch CR31756_4_2 has been updated forcibly by dpasukhi. SHA-1: 1bb778940b1d8a521c964b18518716898f0890ef |
|
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 |
|
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 |
|
Branch CR31756_4_4 has been updated forcibly by dpasukhi. SHA-1: 1fa1ebcfa3b2b1e530eb1fd843e2377cb0f36b50 |
|
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 |
|
Branch CR31756_1 has been deleted by inv. SHA-1: 8c27092e8209ceeeb6a97a5e33a19db099619de0 |
|
Branch CR31756_2 has been deleted by inv. SHA-1: 67ab474f6b69bf2882d1e30b9984d13307ddd9f6 |
|
Branch CR31756_3 has been deleted by inv. SHA-1: a022b5e458ece2597d213fee763e3ad1d3c88ed3 |
|
Branch CR31756_4 has been deleted by inv. SHA-1: a524d2359be3ede0b4807b09739cffa3c7548e0b |
|
Branch CR31756_4_1 has been deleted by inv. SHA-1: 271f53ddd82e3e2b6a5c0c0ee9fcae865a3d8c0e |
|
Branch CR31756_4_2 has been deleted by inv. SHA-1: 1bb778940b1d8a521c964b18518716898f0890ef |
|
Branch CR31756_4_3 has been deleted by inv. SHA-1: b1019f95e9f602a6764b0ccb869871c8dac9a821 |
|
Branch CR31756_4_4 has been deleted by inv. SHA-1: 1fa1ebcfa3b2b1e530eb1fd843e2377cb0f36b50 |
|
Branch CR31756_5 has been deleted by inv. SHA-1: 726f0fe206884aeb0950cd278f86babdbda6b1fc |
|
Branch CR31756_testParser has been deleted by inv. SHA-1: 225b5fa264dcbc01d8edfe2e4c56775917347288 |
occt: master 2057c37b 2020-11-27 19:32:18 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 |
Date Modified | Username | Field | Change |
---|---|---|---|
2020-09-07 13:20 |
|
New Issue | |
2020-09-07 13:20 |
|
Assigned To | => dpasukhi |
2020-09-07 13:20 |
|
Relationship added | related to 0026451 |
2020-09-07 14:39 |
|
File Added: bug31756.stp | |
2020-09-07 14:45 |
|
Description Updated | |
2020-09-07 14:45 |
|
Steps to Reproduce Updated | |
2020-09-17 19:49 |
|
Status | new => assigned |
2020-09-22 18:28 |
|
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 |
|
Note Added: 0097398 | |
2020-12-06 18:25 |
|
Assigned To | abv => dpasukhi |
2020-12-06 18:25 |
|
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 |
|
Note Added: 0097515 | |
2020-12-12 07:44 |
|
Assigned To | abv => bugmaster |
2020-12-12 07:44 |
|
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 |