Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[server] Do not render directory listings for static assets #6764

Merged
merged 3 commits into from
Apr 6, 2016

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Apr 4, 2016

Fixes #6760

@@ -52,6 +52,7 @@ module.exports = () => Joi.object({
disableProtection: Joi.boolean().default(false),
token: Joi.string().optional().notes('Deprecated')
}).default(),
staticDirectoryListings: Joi.boolean().default(Joi.ref('$dev')),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this line do? Make it so that defaults to true when in development and false otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it defaults to the value of $dev, which will only be true when in development mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this implementation, it seems impossible to disable the static directory listings in development. Setting server.staticDirectoryListings: false doesn't seem to have any effect in development.

How about we just default this to false and then people can permanently set it in their kibana.dev.yml if they want to change that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@epixa if the config doesn't work then we have a much bigger issue... I'm giving up on trying to keep it in.

@epixa
Copy link
Contributor

epixa commented Apr 5, 2016

When I run this branch locally (without adjusting my config at all), I get an exception on the server whenever I try to access /bundles/:

TypeError: Path must be a string. Received undefined
    at assertPath (path.js:8:11)
    at posix.join (path.js:479:5)
    at LazyOptimizer.<anonymous> (/p/kibana/src/optimize/lazy/lazy_optimizer.js:69:12)
    at [object Generator].next (native)
    at step (/p/kibana/src/optimize/lazy/lazy_optimizer.js:9:273)
    at run (/p/kibana/node_modules/babel-core/node_modules/core-js/modules/es6.promise.js:104:47)
    at /p/kibana/node_modules/babel-core/node_modules/core-js/modules/es6.promise.js:115:28
    at flush (/p/kibana/node_modules/babel-core/node_modules/core-js/modules/$.microtask.js:19:5)
    at nextTickCallbackWith0Args (node.js:415:9)
    at process._tickDomainCallback (node.js:385:13)

@epixa
Copy link
Contributor

epixa commented Apr 5, 2016

Hmm, actually.. that appears to be happening on master as well.

@epixa
Copy link
Contributor

epixa commented Apr 5, 2016

Well, I can't actually confirm that this works because that exception causes the server to return a 500 error.

@epixa
Copy link
Contributor

epixa commented Apr 5, 2016

It looks like the optimizer is getting into this route definition: https://github.com/spalger/kibana/blob/fix/configureDirectoryListings/src/optimize/lazy/lazy_optimizer.js#L72-L86

But asset is undefined since I'm only trying to access /bundles. I'm not sure what the expected behavior is suppose to be or even how this works in prior versions...

@epixa
Copy link
Contributor

epixa commented Apr 5, 2016

Alright, all of that is a bug, but it's not caused by this PR.

@epixa
Copy link
Contributor

epixa commented Apr 6, 2016

Should we include this config value commented out in the kibana.yml, or do you think it's not important enough?

@epixa epixa assigned spalger and unassigned epixa Apr 6, 2016
@spalger
Copy link
Contributor Author

spalger commented Apr 6, 2016

I really don't want this to go into the config file (by default). I prefer to only have really important stuff there.

This really isn't worth supporting. It's pretty clear that the directory listings are unpopular, and I've never used them or seen them used, so I'm just removing them.

@spalger spalger assigned epixa and unassigned spalger Apr 6, 2016
@epixa
Copy link
Contributor

epixa commented Apr 6, 2016

👍

@epixa
Copy link
Contributor

epixa commented Apr 6, 2016

LGTM

@epixa epixa assigned spalger and unassigned epixa Apr 6, 2016
@spalger spalger merged commit 2fa1760 into elastic:master Apr 6, 2016
@epixa epixa changed the title [server] make directory listing configurable [server] Do not render directory listings for static assets Apr 6, 2016
@spalger spalger deleted the fix/configureDirectoryListings branch October 18, 2019 17:37
cee-chen added a commit that referenced this pull request May 10, 2023
This backport is required to fix a bug with push EUI flyouts before v8.8
release. fixes #157180

## [`77.1.3`](https://github.com/elastic/eui/tree/v77.1.3)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))
jbudz pushed a commit that referenced this pull request May 15, 2023
## Summary

`eui@77.2.2` ⏩ `eui@79.0.1`

🦴 The primary changes in this upgrade are around the deprecated
`EuiLoadingContent` being removed in favor of `EuiSkeletonText`.
- Most instances have been a [direct swap of
usage](327626a),
but [some replacements were a bit more
opinionated](e6ceb36)
as I saw them as potential to take advantage of `EuiSkeletonText`'s
syntactical sugar and screen reader announcements for when state
switches to loaded.

---

## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))


## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0)

- Updated all `EuiSkeleton` components with new props that allow for
more control over screen reader live announcements:
`announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps`
([#6752](elastic/eui#6752))
- Improved keyboard accessibility in `EuiPageHeader` by ensuring the
right side menu items come into focus from left to right.
([#6753](elastic/eui#6753))

**Breaking changes**

- Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton`
components instead. ([#6754](elastic/eui#6754))


## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0)

- Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and
`EuiSwitch` in their unchecked states to meet WCAG AA guidelines.
([#6729](elastic/eui#6729))
- Added React Testing Library `*ByTestSubject` custom commands to
`within()`. RTL utilities can be imported from
`@elastic/eui/lib/test/rtl`.
([#6737](elastic/eui#6737))
- Updated `EuiAvatar` to support a new letter `casing` prop that allow
customizing text capitalization
([#6739](elastic/eui#6739))
- Updated `EuiFocusTrap` to support the `gapMode` prop configuration
(now defaults to `padding`)
([#6744](elastic/eui#6744))

**Bug fixes**

- Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL
and query string generation
([#6717](elastic/eui#6717))
- Fixed `EuiFieldNumber`'s native browser validity detection causing
extra unnecessary rerenders
([#6741](elastic/eui#6741))
- Fixed the `scrollLock` property on `EuiFocusTrap` (and other
components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to
no longer block scrolling on nested portalled content, such as combobox
dropdowns ([#6744](elastic/eui#6744))

**Breaking changes**

- `EuiAvatar`s with the default `user` type will now default to
capitalizing all initials in uppercase
([#6739](elastic/eui#6739))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jasonrhodes pushed a commit that referenced this pull request May 17, 2023
## Summary

`eui@77.2.2` ⏩ `eui@79.0.1`

🦴 The primary changes in this upgrade are around the deprecated
`EuiLoadingContent` being removed in favor of `EuiSkeletonText`.
- Most instances have been a [direct swap of
usage](327626a),
but [some replacements were a bit more
opinionated](e6ceb36)
as I saw them as potential to take advantage of `EuiSkeletonText`'s
syntactical sugar and screen reader announcements for when state
switches to loaded.

---

## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))


## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0)

- Updated all `EuiSkeleton` components with new props that allow for
more control over screen reader live announcements:
`announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps`
([#6752](elastic/eui#6752))
- Improved keyboard accessibility in `EuiPageHeader` by ensuring the
right side menu items come into focus from left to right.
([#6753](elastic/eui#6753))

**Breaking changes**

- Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton`
components instead. ([#6754](elastic/eui#6754))


## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0)

- Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and
`EuiSwitch` in their unchecked states to meet WCAG AA guidelines.
([#6729](elastic/eui#6729))
- Added React Testing Library `*ByTestSubject` custom commands to
`within()`. RTL utilities can be imported from
`@elastic/eui/lib/test/rtl`.
([#6737](elastic/eui#6737))
- Updated `EuiAvatar` to support a new letter `casing` prop that allow
customizing text capitalization
([#6739](elastic/eui#6739))
- Updated `EuiFocusTrap` to support the `gapMode` prop configuration
(now defaults to `padding`)
([#6744](elastic/eui#6744))

**Bug fixes**

- Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL
and query string generation
([#6717](elastic/eui#6717))
- Fixed `EuiFieldNumber`'s native browser validity detection causing
extra unnecessary rerenders
([#6741](elastic/eui#6741))
- Fixed the `scrollLock` property on `EuiFocusTrap` (and other
components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to
no longer block scrolling on nested portalled content, such as combobox
dropdowns ([#6744](elastic/eui#6744))

**Breaking changes**

- `EuiAvatar`s with the default `user` type will now default to
capitalizing all initials in uppercase
([#6739](elastic/eui#6739))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants