Follow-up to #1956698: Prevent access to YAML files using .htaccess and web.config

Problem/Motivation

file_htaccess_lines() is a fairly useless one-line procedural wrapper for a primarily internal function.

Proposed resolution

Deprecate the wrapper and move all relevant information and usages to the wrapped method.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task; there is no functional bug.
Prioritized changes This is a normal followup from a critical issue, #1956698: Prevent access to YAML files using .htaccess and web.config, but otherwise not a prioritized change. There is an extremely minor impact on developer experience through reducing the public API, but not enough to merit the equally minor BC break during the beta.
Disruption Marking the function as deprecated will cause no disruption for core, contrib, or custom modules, and there is only one usage in core to convert (plus a test).

Remaining tasks

Patch needs review.

User interface changes

N/A

API changes

file_access_lines() removed.

CommentFileSizeAuthor
#12 interdiff-deprecate-12.txt1.06 KBxjm
#12 file-2332047-12.patch3.06 KBxjm
#2 remove-file-htaccess-lines.patch3.42 KBxjm

Comments

xjm’s picture

xjm
she/her
Primary language English
commented

Change record draft: https://www.drupal.org/node/2332051

xjm’s picture

xjm
she/her
Primary language English
commented
StatusFileSize
new remove-file-htaccess-lines.patch3.42 KB
xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
commented
Component: base system » file system

marvil07 queued 2: remove-file-htaccess-lines.patch for re-testing.

marvil07’s picture

marvil07 commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +DX

Less globals FTW, specially if they are used once, just re-tested.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

This issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

I think it is fine to replace the usages and deprecate the procedural function for 9.x since the static method on the class already exists.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Remove unneeded file_htaccess_lines() wrapper » Deprecate unneeded file_htaccess_lines() wrapper
xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -DX, -Needs issue summary update +DX (Developer Experience)
StatusFileSize
new file-2332047-12.patch3.06 KB
new interdiff-deprecate-12.txt1.06 KB

Rerolled to deprecate the function instead (the original patch was posted before the beta). Also added the beta evaluation. The DX impact of having one fewer global doesn't justify a BC break during the beta (no matter how small), but we can deprecate it to the same end, and it was a followup from a critical originally, which is a category of issue allowed during the beta.

marvil07’s picture

marvil07 commented
Status: Needs review » Reviewed & tested by the community

Well, at least deprecating is a good step forward now.

Thanks for the new patch!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 99527b1 on 8.0.x 
    Issue #2332047 by xjm: Deprecate unneeded file_htaccess_lines() wrapper

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone’s picture

quietone commented

publish change record