-
Notifications
You must be signed in to change notification settings - Fork 71
switch the SOT for package:mime to Apache httpd.conf #2180
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
base: main
Are you sure you want to change the base?
switch the SOT for package:mime to Apache httpd.conf #2180
Conversation
PR HealthBreaking changes ✔️
This check can be disabled by tagging the PR with Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with
Coverage
|
File | Coverage |
---|---|
pkgs/mime/lib/src/default_extension_map.dart | 💔 Not covered |
pkgs/mime/lib/src/extension.dart | 💚 100 % |
pkgs/mime/tool/download_mime_info.dart | 💔 Not covered |
pkgs/mime/tool/regenerate_tables.dart | 💔 Not covered |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check
.
API leaks ✔️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
Package | Leaked API symbol | Leaking sources |
---|
This check can be disabled by tagging the PR with skip-leaking-check
.
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files |
---|
no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
Files |
---|
pkgs/bazel_worker/benchmark/benchmark.dart |
pkgs/bazel_worker/example/client.dart |
pkgs/bazel_worker/example/worker.dart |
pkgs/benchmark_harness/integration_test/perf_benchmark_test.dart |
pkgs/boolean_selector/example/example.dart |
pkgs/clock/lib/clock.dart |
pkgs/clock/lib/src/clock.dart |
pkgs/clock/lib/src/default.dart |
pkgs/clock/lib/src/stopwatch.dart |
pkgs/clock/lib/src/utils.dart |
pkgs/clock/test/clock_test.dart |
pkgs/clock/test/default_test.dart |
pkgs/clock/test/stopwatch_test.dart |
pkgs/clock/test/utils.dart |
pkgs/coverage/lib/src/coverage_options.dart |
pkgs/html/example/main.dart |
pkgs/html/lib/dom.dart |
pkgs/html/lib/dom_parsing.dart |
pkgs/html/lib/html_escape.dart |
pkgs/html/lib/parser.dart |
pkgs/html/lib/src/constants.dart |
pkgs/html/lib/src/encoding_parser.dart |
pkgs/html/lib/src/html_input_stream.dart |
pkgs/html/lib/src/list_proxy.dart |
pkgs/html/lib/src/query_selector.dart |
pkgs/html/lib/src/token.dart |
pkgs/html/lib/src/tokenizer.dart |
pkgs/html/lib/src/treebuilder.dart |
pkgs/html/lib/src/utils.dart |
pkgs/html/test/dom_test.dart |
pkgs/html/test/parser_feature_test.dart |
pkgs/html/test/parser_test.dart |
pkgs/html/test/query_selector_test.dart |
pkgs/html/test/selectors/level1_baseline_test.dart |
pkgs/html/test/selectors/level1_lib.dart |
pkgs/html/test/selectors/selectors.dart |
pkgs/html/test/support.dart |
pkgs/html/test/tokenizer_test.dart |
pkgs/html/test/trie_test.dart |
pkgs/html/tool/generate_trie.dart |
pkgs/pubspec_parse/test/git_uri_test.dart |
pkgs/stack_trace/example/example.dart |
pkgs/watcher/test/custom_watcher_factory_test.dart |
pkgs/yaml_edit/example/example.dart |
This check can be disabled by tagging the PR with skip-license-check
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request switches the source of truth for MIME type mappings to the Apache httpd mime.types
file. This is a significant and welcome improvement, centralizing the data source and making updates easier. The changes include new tooling to download and process the MIME data, and the package is updated to use the newly generated mappings.
My review includes a few suggestions:
- Correcting a typo in the
CHANGELOG.md
. - Improving the completeness of the
CHANGELOG.md
to include removed extensions. - A fix for the documentation generation script to correctly handle special markdown characters.
- A suggestion to refactor the main logic in the generation script for better readability.
Overall, this is a solid PR that improves the maintainability of the package.
## 2.1.0-wip | ||
|
||
* Switched to using the Apache httpd mime.conf table as the source of truth for | ||
mime types. | ||
|
||
Mime type additions: | ||
- `application/vnd.geogebra.slides` | ||
- `font/collection` | ||
- `image/jxl` | ||
- `image/vnd.dvb.subtitle` | ||
- `video/mp2t` | ||
|
||
Renamed mime types: | ||
- `application/x-font-otf` => `font/otf` | ||
- `application/x-font-ttf` => `font/ttf` | ||
- `application/x-font-woff` => `font/woff` | ||
|
||
Removed mime types: | ||
- `model/vnd.mts` | ||
|
||
Mime types where the default file extnesion changed: | ||
- `application/inkml+xml`, `inkml` => `ink` | ||
- `application/octet-stream`, `so` => `bin` | ||
- `application/onenote`, `onetoc2` => `onetoc` | ||
- `application/pgp-signature`, `sig` => `asc` | ||
- `application/tei+xml`, `teicorpus` => `tei` | ||
- `application/vnd.adobe.fxp`, `fxpl` => `fxp` | ||
- `application/vnd.clonk.c4group`, `c4u` => `c4g` | ||
- `application/vnd.dece.data`, `uvvf` => `uvf` | ||
- `application/vnd.dece.ttml+xml`, `uvvt` => `uvt` | ||
- `application/vnd.eszigno3+xml`, `et3` => `es3` | ||
- `application/vnd.framemaker`, `maker` => `fm` | ||
- `application/vnd.geometry-explorer`, `gre` => `gex` | ||
- `application/vnd.grafeq`, `gqs` => `gqf` | ||
- `application/vnd.ibm.modcap`, `listafp` => `afp` | ||
- `application/vnd.iccprofile`, `icm` => `icc` | ||
- `application/vnd.intercon.formnet`, `xpx` => `xpw` | ||
- `application/vnd.kde.kpresenter`, `kpt` => `kpr` | ||
- `application/vnd.kde.kword`, `kwt` => `kwd` | ||
- `application/vnd.kinar`, `knp` => `kne` | ||
- `application/vnd.koan`, `skt` => `skp` | ||
- `application/vnd.ms-project`, `mpt` => `mpp` | ||
- `application/vnd.palm`, `pqa` => `pdb` | ||
- `application/vnd.quark.quarkxpress`, `qxt` => `qxd` | ||
- `application/vnd.simtech-mindmapper`, `twds` => `twd` | ||
- `application/vnd.stardivision.writer`, `vor` => `sdw` | ||
- `application/vnd.sus-calendar`, `susp` => `sus` | ||
- `application/vnd.symbian.install`, `sisx` => `sis` | ||
- `application/vnd.ufdl`, `ufdl` => `ufd` | ||
- `application/vnd.visio`, `vsw` => `vsd` | ||
- `application/vnd.zul`, `zirz` => `zir` | ||
- `application/x-authorware-bin`, `x32` => `aab` | ||
- `application/x-blorb`, `blorb` => `blb` | ||
- `application/x-cbr`, `cbz` => `cbr` | ||
- `application/x-director`, `w3d` => `dir` | ||
- `application/x-font-type1`, `pfm` => `pfa` | ||
- `application/x-msdownload`, `msi` => `exe` | ||
- `application/x-pkcs12`, `pfx` => `p12` | ||
- `application/x-pkcs7-certificates`, `spc` => `p7b` | ||
- `application/x-zmachine`, `z8` => `z1` | ||
- `application/xv+xml`, `xvml` => `mxml` | ||
- `audio/basic`, snd => `au` | ||
- `audio/mpeg`, mpga => `mp3` | ||
- `audio/vnd.dece.audio`, `uvva` => `uva` | ||
- `image/tiff`, `tif` => `tiff` | ||
- `image/vnd.dece.graphic`, `uvvi` => `uvi` | ||
- `image/x-freehand`, `fhc` => `fh` | ||
- `message/rfc822`, `mime` => `eml` | ||
- `model/mesh`, `silo` => `msh` | ||
- `model/x3d+binary`, `x3dbz` => `x3db` | ||
- `model/x3d+vrml`, `x3dvz` => `x3dv` | ||
- `model/x3d+xml`, `x3dz` => `x3d` | ||
- `text/troff`, `tr` => `t` | ||
- `text/uri-list`, `urls` => `uri` | ||
- `text/x-fortran`, `for` => `f` | ||
- `video/mj2`, `mjp2` => `mj2` | ||
- `video/vnd.dece.hd`, `uvvh` => `uvh` | ||
- `video/vnd.dece.mobile`, `uvvm` => `uvm` | ||
- `video/vnd.dece.pd`, `uvvp` => `uvp` | ||
- `video/vnd.dece.sd`, `uvvs` => `uvs` | ||
- `video/vnd.dece.video`, `uvvv` => `uvv` | ||
- `video/vnd.uvvu.mp4`, `uvvu` => `uvu` | ||
- `video/x-ms-asf`, `asx` => `asf` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very detailed and helpful changelog! One thing that seems to be missing is a section for extensions that were removed from existing MIME types. For example, the .m4b
extension is no longer associated with audio/mp4
. Documenting these removals would be beneficial for users migrating to this new version.
void main(List<String> args) { | ||
final mimeTypes = readMimeTypes(); | ||
|
||
// Make any necessary mime name conversions. | ||
for (final oldMime in mimeConversions.keys) { | ||
final newMime = mimeConversions[oldMime]!; | ||
mimeTypes[newMime] = mimeTypes.remove(oldMime)!; | ||
} | ||
|
||
// Make sure we don't have conflicts with our preferred extension mappings. | ||
final customExtensions = preferredExtensions.values.toSet(); | ||
for (final exts in mimeAdditions.values) { | ||
customExtensions.addAll(exts); | ||
} | ||
for (final entry in mimeTypes.entries.toList()) { | ||
final exts = entry.value; | ||
exts.removeWhere(customExtensions.contains); | ||
if (exts.isEmpty) { | ||
mimeTypes.remove(entry.key); | ||
} | ||
} | ||
|
||
// Add additonal mime types. | ||
for (final entry in mimeAdditions.entries) { | ||
final mime = entry.key; | ||
final exts = entry.value; | ||
|
||
mimeTypes.putIfAbsent(mime, () => []).addAll(exts); | ||
} | ||
|
||
// Use our preferred extensions for specific types. | ||
for (final entry in preferredExtensions.entries) { | ||
final mime = entry.key; | ||
final preferredExtension = entry.value; | ||
|
||
final exts = mimeTypes[mime]!; | ||
exts.remove(preferredExtension); | ||
exts.insert(0, preferredExtension); | ||
} | ||
|
||
// Sort to normalize the table, but preserve the position of the first file | ||
// extension. | ||
final sortedMimeTypes = <String, List<String>>{}; | ||
for (final mime in mimeTypes.keys.toList()..sort()) { | ||
final exts = mimeTypes[mime]!; | ||
final defaultExt = exts.first; | ||
final other = exts.sublist(1)..sort(); | ||
sortedMimeTypes[mime] = [defaultExt, ...other]; | ||
} | ||
|
||
var file = writeMediaTypesFile(sortedMimeTypes); | ||
print('Wrote ${path.relative(file.path)}.'); | ||
|
||
file = updateMediaTypesMarkdown(sortedMimeTypes); | ||
print('Updated ${path.relative(file.path)}.'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main
function is quite long and handles several distinct steps of logic (applying conversions, removing conflicts, adding new types, setting preferred extensions, sorting). To improve readability and maintainability, in accordance with the Dart style guide1, consider refactoring this logic into smaller, well-named private functions. For example, you could have a function that encapsulates all the customization logic.
Style Guide References
Footnotes
-
The style guide recommends optimizing for readability. Breaking down long functions into smaller, more focused ones improves readability. ↩
File updateMediaTypesMarkdown(Map<String, List<String>> mimeToExtensions) { | ||
// Emit the table. | ||
const startMarker = '<!-- start table -->\n'; | ||
const endMarker = '<!-- end table -->\n'; | ||
|
||
final file = File.fromUri(Platform.script.resolve('../doc/media_types.md')); | ||
final contents = file.readAsStringSync(); | ||
final prefix = | ||
contents.substring(0, contents.indexOf(startMarker) + startMarker.length); | ||
final suffix = contents.substring(contents.lastIndexOf(endMarker)); | ||
|
||
final buf = StringBuffer(prefix); | ||
buf.writeln(); | ||
buf.write(''' | ||
| MIME type | Default | Additional | | ||
| ---------------------------------------- | ----------- | ------------------- | | ||
'''); | ||
|
||
for (final mime in mimeToExtensions.keys) { | ||
final exts = mimeToExtensions[mime]!.toList(); | ||
|
||
final defaultExt = exts.first; | ||
exts.remove(defaultExt); | ||
|
||
final additional = exts.join(', '); | ||
|
||
buf.writeln('| ${mime.padRight(40)} | ' | ||
'${defaultExt.padRight(11)} | ' | ||
'${additional.padRight(19)} |'); | ||
} | ||
|
||
buf.writeln(); | ||
buf.write(suffix); | ||
|
||
file.writeAsStringSync('${buf.toString().trim()}\n'); | ||
|
||
return file; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated markdown in doc/media_types.md
will not render correctly for file extensions or MIME types containing underscores (e.g., fe_launch
), as they will be interpreted as emphasis markers. The previous script handled this by escaping special markdown characters. Please consider adding a similar escaping mechanism here.
You could add a helper function like this:
String _markdownEscape(String markdown) {
return markdown.replaceAll('_', r'\_');
}
And then use it when building the table rows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, but from some experimentation, the single underscore in fe_launch is not interpreted as a special markdown charater:
https://gist.github.com/devoncarew/8c7921034c192fa52ad967fbe95aeee8
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This introduces a new script - tool/download_mime_info.dart - which is used to download the latest apache httpd.conf file to the repo. It also updates tool/regenerate_tables.dart read that file, apply some package specific customizations, and generate a new lib/src/media_types.g.dart file.
The implementation of this package uses that new file for mime => file extension mappings and vice versa. This PR does not introduce any API changes to this package. Any new or changed mime types - due to rebasing on top of apache httpd.conf - are called out in the changelog. And as we don't have a canonical source for 'default' file extensions for mime types we just attempt to minimize the deltas people will see wrt default extensions w/ this update.
Before this lands we'll want to review #2165, and then address an issue w/ magic mime numbers and
audio/weba
(it's not a registered mime type). And separate from this PR, we'll want to author a policy wrt updating the mime mappings.Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.