Commit Graph
23 Commits
Author SHA1 Message Date
Mahamadasif Ansari eb0cc88ba5 [FIX] web: log logger warning instead error in assets
Currently, log level error mesage occur when`ValueError` is catched by line [1].

This commit changes 'logger.error' to 'logger.warning' with filename to
log message instead of error since this is not an actual error of the codebase.

[1]-https://github.com/odoo/odoo/blob/3932c46f914c40be4b0629771d1981cdf289df26/addons/web/controllers/binary.py#L131

sentry-4657682601

closes odoo/odoo#158342

Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
2024-03-22 14:47:55 +00:00
Jorge Pinna Puissant afc8dde97a [FIX] base, web: log correctly error in assets
Since [1], when an error exists in an asset, the assets raise a not
found exception. The issue, is that, there is no feedback for the
developer to find the error.

Now, a new console log with the error details is shown.

[1] : bf3b6b0b8b

closes odoo/odoo#141705

Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
2023-11-10 12:33:40 +00:00
Xavier-Do 0a8f326397 [IMP] web: improve missing map reporting
If a .map is missing (outdated) the error message will report

    'min' expected in extension in non debug mode

when the problem is actually that map are not generate through this
route.
If the attachment corresponding to a .map is not found, it
was most likely garbage collected.

Change the message to:

    .map should have been generated through debug assets, (version
    5eff983 most likely outdated)

closes odoo/odoo#141469

Signed-off-by: Jorge Pinna Puissant (jpp) <jpp@odoo.com>
2023-11-10 11:31:42 +00:00
Xavier-Do bf3b6b0b8b [IMP] base, web, *: change url formating
Previously proposed formating was trying to normalize extra in one part
of the path. This means that no extra needed a placeholder.

The implementation was meant to be more generic and extendible since a
part of the logic has to be in website.

A suggestion was made to make it more restricted but explicite by
keeping the url simple in web/controllers/binary.py but adding a
controller in website to add this extra part.

The base extra direction is now in the extension, as the min part.

Initial urls:
/web/assets/{unique}/[{website_id}/][rtl/]{bundle_name}[.min].{extension}

New urls:
/web/assets/[{website_id}]/{unique}/{bundle_name}[.rtl][.min].{extension}

Managed by two routes:

/web/assets/<string:unique>/<string:filename>
/web/assets/<int:website_id>/<string:unique>/<string:filename>

Where filename is in the format {bundle_name}[.rtl][.min].{extension}

Multiple possibilities where proposed

- /web/assets/website/<int:website_id>/<string:unique>/<string:filename>
More explicit but prefixing by /website was considered

- /website/assets/<int:website_id>/<string:unique>/<string:filename>
This one is a litle painfull to match similar attachement, where
website is ignored.

- /website/<int:website_id>/assets/<string:unique>/<string:filename>
Almost accepted but subjective, and anyway two previous solution breaks
the cdn mecanism and would need a migration

- /web/assets/<int:website_id>/<string:unique>/<string:filename>
Almost accepted but subjective, and anyway two previous solution breaks
the cdn mecanism and would need a migration

This last solution was not ideal to match without unique
/web/assets/%/<string:filename> can match both

/web/assets/123456/<string:filename>
and
/web/assets/1/123456/<string:filename>

Anyway, matching without unique shouldn't be supported for al (even if
it is kind of supported with any right now) but it will work by changing
unique wildcard to a more specific one (_ * 7)

closes odoo/odoo#131353

Related: odoo/enterprise#47313
Related: odoo/design-themes#730
Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
2023-10-27 11:34:52 +00:00
Xavier-Do 94a47f25ce [IMP] web, website: add validation for /web/assets
Increase the validation of assets url especially when parsing extra to
avoid aving to much possible url for the same asset.

Part-of: odoo/odoo#131353
2023-10-27 11:34:52 +00:00
Xavier-Do 64bbef5bc1 [IMP] base, web: generate assets outside rendering
The generation inside the rendering has some drawbacks:

- `commit_assetsbundle` is needed for reports rendering because the
template rendering may generate some assets that will be accessed by
another transaction before the transaction is committed. But this
solution is not ideal since the transaction is committed in the middle
of the request

- when the first rendered page is a 404, the assets are not committed
and the page is broken.

- when starting, deleting an attachment can create a concurrent update
error and the request is retried. This will occur once per attachment
and for all worker trying to access the same resource. The whole
transaction is rollbacked, even the previously created assets bundle.

- The cold page load is a slower since there is more work to do.

- Implementing a readonly request is difficult because it could be
transformed to read write and re-executed if the assets bundle does not
exist.

Generating assets when needed solves those issues. The concurrency
when deleting an assets could still occur but only once per bundle, and
in a smaller transaction. This could be solved with a lock now that we
have more control on the transaction. The commit_assetsbundle can be
removed and 404 page should have a correct layout. The cold page load
could be a little faster because the assets bundle can be generated in
parallel requests instead of sequentially when rendering the page.

Part-of: odoo/odoo#131353
2023-10-27 11:34:52 +00:00
Xavier-Do d988030134 [REF] base: don't add id to assets bundle url
The main motivation is to be able to generate assets bundle outside
the t-call-assets call.
The need of an id in the url makes it mandatory to have an attachment
when adding the url in the page. Without this restriction, we can guess
the url without generating the assets.

This can also have other useful side effect:
There are corner case when a worked could have an invalid url in
cache because, if the transaction is rollbacked or if another request
generates the same attachment at the same time. This should be
partially solved by removing the id: The url remains valid even if the
attachment does not exist.

Note that the extra part of the url was made explicit, always there and
taking one / to remove complexity and ambiguity.

Note that an additional query appeared in .test_50_perf_sql_web_assets
because of the search, this but two of them were in _find_record. One of
them was an `exist`, not making much sense since we are not getting the
id from the attachment url anymore but from a search, and the other one
was prefetch of the "public field" since the call to _find_record does
not go in other cases (xmlid, website published, access token, ....). A
attachment of a asset is always public, and this part of the security
was moved to the search domain. The final result is one less query:
- one query to search
- one query to read the fields (_get_stream_from) (the prefetch could
actually be set to avoid prefetching everything)

Part-of: odoo/odoo#131353
2023-10-27 11:34:52 +00:00
Martin Trigaux 22ab49e343 [IMP] *: use file_path and file_open
Replace all the calls to get_resource_path to the better file_path or
directly use file_open when not needed

Doing both a get_resource_path and file_open means checking twice that
the file exists.
Doing a simple path concatenation before a file_open is safe.
If given to another method (e.g. etree.parse), calling file_path is
the prefered method.

Note that get_resource_path used to return False when the file does
not exists while file_path/file_open raises a FileNotFoundException

closes odoo/odoo#135607

Related: odoo/upgrade#5187
Related: odoo/enterprise#47475
Signed-off-by: Martin Trigaux (mat) <mat@odoo.com>
2023-10-06 14:33:43 +00:00
Julien Castiaux cc5a14b6a9 [FIX] core: prevent upload of large files
The limit was only enforced by the front-end, meaning that anybody could
forge a request with a huge file and get it processed by Odoo. According
to the documentation of werkzeug[^1], such limit should be enforced by
the server server instead of the wsgi application. It is the case for
Odoo Online but on-premise customers might not configure their servers.

The `web.max_file_upload_size` system paramter is now enforced upon
parsing the content of the request. It defaults at 128 MiB which is
enough for most documents and images. We do not want to host large
files (e.g. videos) in the Odoo filestore.

[^1]: https://werkzeug.palletsprojects.com/en/2.0.x/request_data/

Fixes: #124646
Part-of: odoo/odoo#126914
2023-08-11 14:32:00 +02:00
flvr-odoo a5f8788332 [FIX] web : adding csp for automated scanner
Due to numerous client complaining about their automated scanner not
seeing the CSP header on the http request, we add it for good mesures

opw-2793379

closes odoo/odoo#121098

X-original-commit: 70a54ac604cf2aeef33d11b95851e35dd463727f
Signed-off-by: Vranckx Florian (flvr) <flvr@odoo.com>
2023-05-11 00:33:48 +02:00
nda-odoo f5e8d8ea92 [FIX] web: faster asset search
Help postgres planner avoiding a scan over the entire table.

On a database with 5.5M ir_attachment but only a few url:
before: https://explain.dalibo.com/plan/g45f7492a78d6e19#raw
after: https://explain.dalibo.com/plan/1f5868ba6909b5df#raw

from 1156.316 ms to 2.334 ms

opw-3134913

closes odoo/odoo#113389

X-original-commit: 1549c4dc81f1f5f327b9115944faa87255c31624
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
2023-02-22 17:48:58 +01:00
Benoit Socias fb9efde4f3 [FIX] *: replace werkzeug's Response by odoo's Response
*: base, http_routing, mass_mailing, web, web_editor, website_slides

In some situations `werkzeug.wrappers.Response` are used instead of
`odoo.http.Reponse` that extends it.
This is a problem because since [1] the calls to `set_cookie` expect it
to accept the `cookie_type` parameter, which is not the case in the base
werkzeug implementation.

This commit replaces the `werkzeug.wrappers.Response` by
`odoo.http.Response`.

[1]: https://github.com/odoo/odoo/commit/2cbda6c98ee947cea1d06c09880eee8c758304a8

closes odoo/odoo#112827

X-original-commit: 28da08292b7028575e628c5ad846fc05d30498f2
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
2023-02-16 09:01:07 +01:00
Hubert Van de Walle (huvw) fdd60894ae [FIX] web: rtl assets in ltr language in debug mode
Steps to reproduce:

  - Switch to `?debug=assets`
  - Change the user language to Arabic and back to English
  -> The page is still displayed in rtl mode

Cause of the issue:

  The css is retrieved like this

  ```py
  >>> self.env['ir.attachment'].sudo().search([('url', '=like', '/web/assets/%/web.assets_common.css')])
  ir.attachment(212, 189)

  >>> self.env['ir.attachment'].sudo().search([('url', '=like', '/web/assets/%/web.assets_common.css')]).mapped('url')
  ['/web/assets/212-5d47380/rtl/web.assets_common.css', '/web/assets/189-5d47380/web.assets_common.css']
  ```
  Only the second one should be matched.

Solution:

  Check for the absence of an extra parameter in the url

opw-2892012

closes odoo/odoo#105127

X-original-commit: 539427fa2a099b29adf099c2b48d4f1d2fd4ebd2
Signed-off-by: Julien Castiaux <juc@odoo.com>
Signed-off-by: Hubert Van De Walle <huvw@odoo.com>
2022-11-17 10:57:10 +01:00
Rémy Voet (ryv)andJulien Castiaux e967e25095 [IMP] *: remove bad usage of search_read.
Reading only 'id' with `search_read` is equivalent to use `search` but
complexify the result usage. Fix all these bad usages.

Part-of: odoo/odoo#104838
Co-authored-by: Julien Castiaux <juc@odoo.com>
2022-11-07 11:26:32 +01:00
Yolann Sabaux 19ed75ddab [FIX] mail: revert upload_attachment fix
Cookies should be used internally by the web UI. The server-side is not supposed to be aware of it at all.

Reverts:
odoo#88745

Based on odoo#93812
discussion. It has been decided to revert the fix to avoid further unattended behaviours.

closes odoo/odoo#100178

X-original-commit: bdde7dda7356744d459e3991a7f382fee42bf8c5
Signed-off-by: Nicolas Lempereur (nle) <nle@odoo.com>
Signed-off-by: Yolann Sabaux (yosa) <yosa@odoo.com>
2022-09-15 10:03:09 +02:00
Gorash 48c267e9a5 [IMP] web/http: Added support for immutable property to serve files
When a file is immutable the different network layers can cache it.
Services receiving this header should never invalidate these files.

Part-of: odoo/odoo#95500
2022-09-14 20:25:00 +02:00
Pierre-Yves Dufays 390c36f42f [FIX] web: makes the logo route return the correct image
Emails got always the odoo logo even if the company logo has been changed. This
solves the problem.

Technical note: the problem was caused by an exception in the controller due to
invalid parameters used for send_file method causing web/static/img/nologo.png
to be returned.

Task-2920690

closes odoo/odoo#99073

Signed-off-by: Julien Castiaux <juc@odoo.com>
2022-09-09 16:23:40 +02:00
Julien Castiaux 4960665043 [FIX] web: missing cache of /web/assets
Fine tuning of da8def8e41

closes odoo/odoo#93407

X-original-commit: 7efa8743b1bbe9efc4b81a10331f096e8de1e52d
Signed-off-by: Julien Castiaux <juc@odoo.com>
2022-06-13 13:31:47 +02:00
Julien Castiaux fc115ebdf7 [FIX] web: invalid image placeholder resizing
Access `/web/image/82303?height=16`, traceback because the placeholder
image cannot be resized to `"16"`.

closes odoo/odoo#92891

Signed-off-by: Julien Castiaux <juc@odoo.com>
2022-06-03 18:34:49 +02:00
Julien Castiaux da8def8e41 [IMP] core, web: Delegate delivery of static files
Rationnals
----------

Web servers can serve some resources (e.g. static files) right away
without any interaction with the web application. The network model of
most web servers makes them capable of handling thousands of
simultaneous requests when it comes to intensive IO operations such as
streaming data from a file. The network model of Odoo is different: it
is capable of a lot of processing power but can only serve a handful of
requests at a time, i.e. Odoo (with some help from postgres) is
optimized for CPU operations, not IO.

Some users don't configure their web server, they use a basic
configuration that relay all requests to Odoo. The result is that many
Odoo HTTP Workers can be busy streaming static files instead of
processing other requests. This can lead to a worker starvation, i.e.
all workers are busy streaming files and cannot process new requests.

X-Sendfile
----------

In this work, we add the support for the [X-Sendfile] header family,
they are multiples http headers that can be used by the web application
to communicate with the web server in order to delegate the delivery of
files stored on the file system. Odoo still receives the request but it
does no more stream the file content from within its HTTP worker,
instead it skips the response body altogether and sets the `X-Sendfile`
special header with the path of the file on the filesystem. The web
server intercepts that special header, open the file and stream it.

Using those headers, we can use the best of both the web application and
the web server. The web application is still responsible to locate the
resource and verify the access rights, the web server is still
responsible of streaming the content.

Using X-Sendfile is opt-in via the `--x-sendfile` CLI flag. We set both
`X-Sendfile` (apache) and `X-Accel-Redirect` (nginx). If you are using
apache, make sure `mod_xsendfile` is enabled. If you are using NGINX
you have to add the following location block:

    location /web/filestore {  # custom path, hardcoded within Odoo
        # Prevent access from the outside world, i.e. makes this
        # route only accessible via X-Accel. MANDATORY!!!
        internal;

        # Give access to the filestore using this server's
        # permissions. Odoo is in charge of verifying the access
        # rights.
        alias /path/to/odoo/data-dir/filestore;
    }

The Odoo [deployment documentation] has been updated accordingly.

[X-Sendfile]: https://www.nginx.com/resources/wiki/start/topics/examples/xsendfile/
[deployment documentation]: https://www.odoo.com/documentation/master/administration/install/deploy.html#serving-static-files-and-attachments

Changes to the API
------------------

To benefit most from X-Sendfile, all APIs related to streaming content
over HTTP has to be adapted. They are: (1) `request._serve_static`,
(2) `ir.http._serve_fallback`, (3) `/web/content` and (4) `/web/image`.

Each used it own way to deliver content: (1) `_serve_static` was using
`send_file` (flask's send_file that as been vendored with odoo 10
years ago and not maintenained since then), (2) _serve_fallback was
handcrafting a `werkzeug.wrappers.Response`, (3) /web/content-image were
using the "binary server" `ir.http.binary_content` API.

I has been decided to remove all 3 APIs and to merge the code inside of
the new `http.Stream` object and the `ir.binary` helper model.

A Stream wraps what is going to be sent to the browser, it can be a path
to a file on the locale filesystem, a blob of raw data or an URL to an
external resource. The Stream also holds various metadata that are
mainly used for caching. The preferred way to create a Stream is via one
of its three factories so that all the metadata are set. The factories
are: `from_path`, `from_attachment` and `from_binary_field`. A stream
instance exposes a single method `get_response()` used to create the
corresponding HTTP response object out of the stream.

Inside of `ir.http` were a few methods that were not related to the http
routing and formed what was called the "binary server". All those
methods have been removed and the feature have been refactored inside of
the new `ir.binary` model. The removed methods are:

- `_xmlid_to_obj`
- `_get_record_and_check`
- `_binary_ir_attachment_redirect_content`
- `_binary_record_content`
- `_binary_set_headers`
- `binary_content`
- `_response_by_status`
- `_get_content_common`
- `_content_image`
- `_content_image_get_response`
- `_placeholder_image_get_response`

The new `ir.binary` abstract model exposes the following utilities:

**`_find_record`**

Find an attachment or a record with a binary-field out of an xmlid or
out of a pair record-model/record-id. Check the access rights and the
access token.

**`_get_stream_from`**

Create a Stream from an attachment or a record with a binary-field.

**`_get_image_stream_from`**

Same as `_get_stream_from` but adapted for images. It sets a sensible
ETag on the stream and has image resizing support.

**`_placeholder`**

Get the image placeholder blob.

Testing
-------

It is possible to test the web server configuration using the
`test_http` module. Install the module then run the unittest using the
`webserver` test-tag. By default it attempts to connect to a web-server
running on `http://localhost:80`, you can change this URL by setting the
`WEB_SERVER_URL` environment variable.

    odoo-bin -i test_http --stop-after-init
    WEB_SERVER_URL='http://localhost:80' odoo-bin --test-tags webserver --stop-after-init

closes odoo/odoo#88134

Task: 2801675
Related: odoo/documentation#2083
Related: odoo/enterprise#26191
Signed-off-by: Julien Castiaux <juc@odoo.com>
2022-06-01 02:53:59 +02:00
Yolann Sabaux dc8cf87fc8 [FIX] web: fetch right company_id
Steps to reproduce:
- Have two companies set up
- In settings, check for company 2 the Files Centralization
- For a Product, upload a document

Issue:
The document will not appear in Documents.
It will only appear if the option is checked for company 1

Cause:
The company_id is not fetched correctly throughout the process.
There is a similar solution for the specific `documents` upload route:
https://github.com/odoo/enterprise/blob/bdf712d66c3e5cee70a6b424b69a618fe655a39f/documents/controllers/main.py#L147-L149

Solution:
Get the id directly from the cookies

opw-2774365

closes odoo/odoo#91751

X-original-commit: 46db92d5211c215d07448676e619154390b7147f
Signed-off-by: Nicolas Lempereur (nle) <nle@odoo.com>
Signed-off-by: yosa-odoo <yosa@odoo.com>
2022-05-19 13:45:05 +02:00
Nguyen Viet Phuong dfe70c656b [FIX] web, mail: error when adding attachments
Fix errors when adding attachment to records with read only permission:

Current behavior:
1. Error adding attachment to records with read only permission.
2. The message is not as clear as version 13.0

closes odoo/odoo#88036

Expect: Only display the message "You are not allowed to upload an attachment here." if you do not have permission to upload files.
X-original-commit: 8385f192873cfd33f311e00d9074571f1b3810a7
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
2022-04-05 19:18:37 +02:00
Julien Castiaux bcf665a291 [MOV] web: split controllers.main in several files
The odoo.addons.web.controllers.main module was a very long bloated file
where many different controllers were concatened. It proved difficult to
work on that file on a regular basis,mainly because ctrl-p "web main.py"
was not pointing the right file.

In this work the file has been split on the basic 1 controller = 1 file.
The original way of importing stuff (through main.py) is still possible
thanks to deprecated aliases.

Part-of: odoo/odoo#87571
2022-03-31 02:10:53 +02:00