[wp-trac] [WordPress Trac] #26829: Use of strpos() in extract_from_markers() and insert_with_markers() can target wrong BEGIN and END markers.

[WordPress Trac]

#26829: Use of strpos() in extract_from_markers() and insert_with_markers() can
target wrong BEGIN and END markers.
-------------------------------------------------+-------------------------
 Reporter:  Faison                               |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  5.2
Component:  Rewrite Rules                        |     Version:  2.0
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-      |     Focuses:
  refresh                                        |
-------------------------------------------------+-------------------------
Changes (by pento):

 * keywords:  has-patch has-unit-tests => has-patch has-unit-tests needs-
     refresh
 * owner:  pento => (none)


Comment:

 After reviewing this some more, I like the direction of
 [attachment:"26829.refreshed.3.diff"], I don't think we need to provide
 the ability to mock the filesystem classes, as [attachment:"26829.2.diff"]
 does.

 There are a few issues with [attachment:"26829.refreshed.3.diff"]  that
 need to be addressed:
 - Using the `$wp_filesystem` global would be better than the direct file
 access that `insert_with_markers()` currently does.
 - Does the `FILE_IGNORE_NEW_LINES` flag allow for `\r\n` newlines? If not,
 we should be stripping them.
 - The behaviour in Core allows for the markers to be indented. `trim()`ing
 the line before comparing it will allow for that, too.

 More unit tests for different line endings and indenting would be good,
 too.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/26829#comment:17>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform