Summary

Add a method to check if a mimetype is supported and improve guess_mimetype.

Review Request #14310 — Created Jan. 27, 2025 and submitted Feb. 10, 2026, 8:38 p.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This adds a method for checking whether a mimetype is supported or not.

This is useful for code that deals with uploading binary files, to help

decide which files should be uploaded. We have similar logic for this in

RBTools. Valid and invalid mimetypes are saved to the cache and are

checked against when available, which should help speed things up when

the caller is checking a sequence of mimetypes.
This change also fixes a fault with guess_mimetype. The function runs

the file command in a subprocess to determine the given file's mimetype.

The function doesn't properly close and clean up the subprocess's resources

(the file descriptors for stdin and stdout). In the Review Board server

process which runs for a long time, we're relying on Python's garbage

collector to close everything. The garbage collector is not deterministic

and it could take a while before the resources are cleaned up. It's better

to deal with these ourselves to avoid any unexpected side effects. This

change makes sure the resources get cleaned up when they're no longer used.

Testing Done


Ran unit tests.
Used in the Bitbucket Server Pull Request integration code in Power

  Pack.
Saw the ResourceWarning: unclosed file coming from guess_mimetype

  go away in the logs.

Commits

Summary	ID
Add a method to check if a mimetype is supported and improve guess_mimetype. This adds a method for checking whether a mimetype is supported or not. This is useful for code that deals with uploading binary files, to help decide which files should be uploaded. We have similar logic for this in RBTools. Valid and invalid mimetypes are saved to the cache and are checked against when available, which should help speed things up when the caller is checking a sequence of mimetypes. This change also fixes a fault with `guess_mimetype`. The function runs the `file` command in a subprocess to determine the given file's mimetype. The function doesn't properly close and clean up the subprocess's resources (the file descriptors for stdin and stdout). In the Review Board server process which runs for a long time, we're relying on Python's garbage collector to close everything. The garbage collector is not deterministic and it could take a while before the resources are cleaned up. It's better to deal with these ourselves to avoid any unexpected side effects. This change makes sure the resources get cleaned up when they're no longer used.	dd965637ff55487c8ee49a120c19ef6aa04185ab

Summary

Add a method to check if a mimetype is supported and improve guess_mimetype.

This adds a method for checking whether a mimetype is supported or not. This is useful for code that deals with uploading binary files, to help decide which files should be uploaded. We have similar logic for this in RBTools. Valid and invalid mimetypes are saved to the cache and are checked against when available, which should help speed things up when the caller is checking a sequence of mimetypes. This change also fixes a fault with `guess_mimetype`. The function runs the `file` command in a subprocess to determine the given file's mimetype. The function doesn't properly close and clean up the subprocess's resources (the file descriptors for stdin and stdout). In the Review Board server process which runs for a long time, we're relying on Python's garbage collector to close everything. The garbage collector is not deterministic and it could take a while before the resources are cleaned up. It's better to deal with these ourselves to avoid any unexpected side effects. This change makes sure the resources get cleaned up when they're no longer used.

dd965637ff55487c8ee49a120c19ef6aa04185ab

Issues

Description	From	Last Updated
Maybe indent this as: ... stdin=subprocess.PIPE, ) as p: assert p.stdin is not None ... to make the block clearer?	david	Jan. 28, 2025, 10:47 a.m.
This blank line should go away now.	david	Jan. 28, 2025, 10:48 a.m.
I feel like this might be simplified if we did individual cache entries with a key like review-uis:supported-mimetype:{mimetype} and then …	david	Jan. 28, 2025, 10:55 a.m.
It might be nice to have some in-memory caching (like a global) for this, so we don't have to re-parse …	david	Jan. 28, 2025, 11 a.m.
Blank line after this.	david	Jan. 28, 2025, 11:01 a.m.
I feel like this file would be better served using cache_memoize(), since that's designed for this kind of use case.	chipx86	Feb. 8, 2026, 8:14 p.m.

flake8 passed.

JSHint passed.

reviewboard/attachments/mimetypes.py (Diff revision 1)

The issue has been resolved. Show all issues

Maybe indent this as:

    ...
    stdin=subprocess.PIPE,
) as p:
    assert p.stdin is not None
    ...

to make the block clearer?

reviewboard/attachments/mimetypes.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This blank line should go away now.
```

reviewboard/attachments/mimetypes.py (Diff revision 1)

The issue has been resolved. Show all issues

I feel like this might be simplified if we did individual cache entries with a key like review-uis:supported-mimetype:{mimetype} and then just stored a True or False.

reviewboard/attachments/mimetypes.py (Diff revision 1)

The issue has been resolved. Show all issues

It might be nice to have some in-memory caching (like a global) for this, so we don't have to re-parse the mime types if this gets called repeatedly.

reviewboard/attachments/tests/test_mimetypes.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line after this.
```

Commits:

	Summary	ID
	Add a method to check if a mimetype is supported and improve guess_mimetype. This adds a method for checking whether a mimetype is supported or not. This is useful for code that deals with uploading binary files, to help decide which files should be uploaded. We have similar logic for this in RBTools. Valid and invalid mimetypes are saved to the cache and are checked against when available, which should help speed things up when the caller is checking a sequence of mimetypes. This change also fixes a fault with `guess_mimetype`. The function runs the `file` command in a subprocess to determine the given file's mimetype. The function doesn't properly close and clean up the subprocess's resources (the file descriptors for stdin and stdout). In the Review Board server process which runs for a long time, we're relying on Python's garbage collector to close everything. The garbage collector is not deterministic and it could take a while before the resources are cleaned up. It's better to deal with these ourselves to avoid any unexpected side effects. This change makes sure the resources get cleaned up when they're no longer used.	28b5db79689bfe033eb31a4be91b56ae6c771903
	Add a method to check if a mimetype is supported and improve guess_mimetype. This adds a method for checking whether a mimetype is supported or not. This is useful for code that deals with uploading binary files, to help decide which files should be uploaded. We have similar logic for this in RBTools. Valid and invalid mimetypes are saved to the cache and are checked against when available, which should help speed things up when the caller is checking a sequence of mimetypes. This change also fixes a fault with `guess_mimetype`. The function runs the `file` command in a subprocess to determine the given file's mimetype. The function doesn't properly close and clean up the subprocess's resources (the file descriptors for stdin and stdout). In the Review Board server process which runs for a long time, we're relying on Python's garbage collector to close everything. The garbage collector is not deterministic and it could take a while before the resources are cleaned up. It's better to deal with these ourselves to avoid any unexpected side effects. This change makes sure the resources get cleaned up when they're no longer used.	1e49515439845acb8737028c2d9d4f7cc517133b

Diff:

Revision 2 (+286 -62)

Show changes

	reviewboard/attachments/mimetypes.py
	reviewboard/attachments/tests/test_mimetypes.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

reviewboard/attachments/mimetypes.py (Diff revision 2)

The issue has been resolved. Show all issues

I feel like this file would be better served using cache_memoize(), since that's designed for this kind of use case.

Change Summary:

Use cache_memoize instead of manually dealing with caching.

Commits:

	Summary	ID
	Add a method to check if a mimetype is supported and improve guess_mimetype. This adds a method for checking whether a mimetype is supported or not. This is useful for code that deals with uploading binary files, to help decide which files should be uploaded. We have similar logic for this in RBTools. Valid and invalid mimetypes are saved to the cache and are checked against when available, which should help speed things up when the caller is checking a sequence of mimetypes. This change also fixes a fault with `guess_mimetype`. The function runs the `file` command in a subprocess to determine the given file's mimetype. The function doesn't properly close and clean up the subprocess's resources (the file descriptors for stdin and stdout). In the Review Board server process which runs for a long time, we're relying on Python's garbage collector to close everything. The garbage collector is not deterministic and it could take a while before the resources are cleaned up. It's better to deal with these ourselves to avoid any unexpected side effects. This change makes sure the resources get cleaned up when they're no longer used.	1e49515439845acb8737028c2d9d4f7cc517133b
	Add a method to check if a mimetype is supported and improve guess_mimetype. This adds a method for checking whether a mimetype is supported or not. This is useful for code that deals with uploading binary files, to help decide which files should be uploaded. We have similar logic for this in RBTools. Valid and invalid mimetypes are saved to the cache and are checked against when available, which should help speed things up when the caller is checking a sequence of mimetypes. This change also fixes a fault with `guess_mimetype`. The function runs the `file` command in a subprocess to determine the given file's mimetype. The function doesn't properly close and clean up the subprocess's resources (the file descriptors for stdin and stdout). In the Review Board server process which runs for a long time, we're relying on Python's garbage collector to close everything. The garbage collector is not deterministic and it could take a while before the resources are cleaned up. It's better to deal with these ourselves to avoid any unexpected side effects. This change makes sure the resources get cleaned up when they're no longer used.	dd965637ff55487c8ee49a120c19ef6aa04185ab

Branch:

release-7.x

release-7.1.x

Diff:

Revision 3 (+274 -60)

Show changes

	reviewboard/attachments/mimetypes.py
	reviewboard/attachments/tests/test_mimetypes.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.1.x (5bcb517)