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

Trouble loading assets from localhost while doing development #11533

Closed
sbesh91 opened this issue Jan 26, 2024 · 0 comments · Fixed by #11543
Closed

Trouble loading assets from localhost while doing development #11533

sbesh91 opened this issue Jan 26, 2024 · 0 comments · Fixed by #11543
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior

Comments

@sbesh91
Copy link

sbesh91 commented Jan 26, 2024

Bevy version

V12.1

What you did

I've been trying to run

asset_server.load("some http url from localhost with a port")

What went wrong

I've been working with this library using an image pulled from S3 as a testing URL.

This image parses in just fine, but I'm running into an asset path parsing error when sending in a url that contains a port.

Ideally, I'd like to use pre-signed urls from S3 and that would look something like this.

The error I'm seeing so far looks like this.

/bevy_asset-0.12.1/src/path.rs:104:37: called `Result::unwrap()` on an `Err` value: InvalidSourceSyntax

I tracked down where I think this is being thrown to the while loop that parses paths looking for :. Since there is a second : in the url containing a path, it seems to throw that InvalidSourceSyntax error.

    while let Some((index, char)) = chars.next() {
          match char {
              ':' => {
                  let (_, char) = chars
                      .next()
                      .ok_or(ParseAssetPathError::InvalidSourceSyntax)?;
                  if char != '/' {
                      return Err(ParseAssetPathError::InvalidSourceSyntax);
                  }
                  let (index, char) = chars
                      .next()
                      .ok_or(ParseAssetPathError::InvalidSourceSyntax)?;
                  if char != '/' {
                      return Err(ParseAssetPathError::InvalidSourceSyntax);
                  }
                  source_range = Some(0..index - 2);
                  path_range.start = index + 1;
              }
              '#' => {
                  path_range.end = index;
                  label_range = Some(index + 1..asset_path.len());
                  break;
              }
              _ => {}
          }
      }
@sbesh91 sbesh91 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jan 26, 2024
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds and removed S-Needs-Triage This issue needs to be labelled labels Jan 26, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 26, 2024
# Objective


Fixes #11533 


When `AssetPath`s are created from a string type, they are parsed into
an `AssetSource`, a `Path`, and a `Label`.
The current method of parsing has some unnecessary quirks:

- The presence of a `:` character is assumed to be the start of an asset
source indicator.
- This is not necessarily true. There are valid uses of a `:` character
in an asset path, for example an http source's port such as
`localhost:80`.
- If there are multiple instances of `://`, the last one is assumed to
be the asset source deliminator.
- This has some unexpected behavior. Even in a fully formed path, such
as `http://localhost:80`, the `:` between `localhost` and `80` is
assumed to be the start of an asset source, causing an error since it
does not form the full sequence `://`.


## Solution
Changes the `AssetPath`'s `parse_internal` method to be more permissive.
- Only the exact sequence `://` is taken to be the asset source
deliminator, and only the first one if there are multiple.
- As a consequence, it is no longer possible to detect a malformed asset
source deliminator, and so the corresponding error was removed.
tjamaan pushed a commit to tjamaan/bevy that referenced this issue Feb 6, 2024
# Objective


Fixes bevyengine#11533 


When `AssetPath`s are created from a string type, they are parsed into
an `AssetSource`, a `Path`, and a `Label`.
The current method of parsing has some unnecessary quirks:

- The presence of a `:` character is assumed to be the start of an asset
source indicator.
- This is not necessarily true. There are valid uses of a `:` character
in an asset path, for example an http source's port such as
`localhost:80`.
- If there are multiple instances of `://`, the last one is assumed to
be the asset source deliminator.
- This has some unexpected behavior. Even in a fully formed path, such
as `http://localhost:80`, the `:` between `localhost` and `80` is
assumed to be the start of an asset source, causing an error since it
does not form the full sequence `://`.


## Solution
Changes the `AssetPath`'s `parse_internal` method to be more permissive.
- Only the exact sequence `://` is taken to be the asset source
deliminator, and only the first one if there are multiple.
- As a consequence, it is no longer possible to detect a malformed asset
source deliminator, and so the corresponding error was removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants