Switching to the New Module Library

Module Discussion
225

Great. A few questions

  • Is the match supposed to be exact? E.g., will module version 1.2.3-alpha match only release 1.2.3-alpha, or will it also match 1.2.3? It seems that the match must be exact. If so, is that really what you want? I mean, you could have a future release, say 1.2.3 to which you upload a pre-release 1.2.3-alpha, or a release 1.2.3 to which you upload a specific build 1.2.3+french.

  • Should module extensions (.vext) not also be checked against the release?

  • What about saves (.vsav), logs (.vlog), and older extensions (.vmdx)? Shouldn’t they be checked too?

  • It seems that checking the release number depends on the uploaded file name ending in either .vmod or .vext. Shouldn’t the check be done based on the content (mime-type) of the file? E.g., similar to what you have in image.rs for images:

    • Check if file is mime-type application/zip
      • Check if archive contains moduledata
        • If so, investigate data/version in the XML

    Currently, a user could upload a module which file name ends in say .vmd and thereby by-pass the release check.

Again, with the caveat that I’m not too familiar with Rust

pub fn is_magic_zip(mime: &Mime) -> bool {
    mime == &mime::APPLICATION && mime.sub_type() == "zip";
}
fn is_zip_archive<P: AsRef<Path>>(
    path: P,
    buf: &[u8]
) -> bool {
    match infer::get(buf) {
        Some(info) =>
            info.mime_type().parse::<Mime>()
                .is_ok_and(|mime| is_magic_zip(&mime)),
        None => false
    }
}
fn moduledata_in_zip<P: AsRef<Path>>(
    path: P) -> Result<String, Error>
{
    let zipfile = File::open(zippath)?;
    let mut archive = ZipArchive::new(zipfile)?;

    // check for moduledata in archive
    let mut file = archive.by_name("moduledata");

    if ! file {
        return Ok("");
    } 

    let mut data = String::new();
    file.read_to_string(&mut data)?;
    Ok(data)
}

// Return empty string if not VASSAL file (zip archive and contains moduledata)
// otherwise, the version number set in moduledata
fn check_for_vassal_file(<P: AsRef<Path>>(
    path: P,
    buf: &[u8]
) -> Result<String,Error> {
     if ! is_zip_archive(path,buf) {
         return "";
    }

   let md = moduledata_in_zip(path)?;
   if md == "" {
         return "";
   }
   let package = sxd_document::parser::parse(md)?;
   let document = package.as_document();
   let value = sxd_xpath::evaluate_xpath(&document, "/data/version")?;
   Ok(value.string())
}

and then in line 326 of prod_core.rs

        let version = check_for_vassal_file(file.file_path(),file)?;
        if version != "" {
            let mod_version = version.parse::<Version>()?; // .await perhaps?
            let rel_version = self.db.get_release_version(release).await?;
            if mod_version != rel_version {
                return Err(AddFileError::ReleaseVersionMismatch(
                    mod_version, rel_version
                ));
            }
        }

Oh, and in line 371 of GameSection perhaps add the paragraph

<p>
  Game box images can be  PNG, GIF, JPEG, SVG, AVIF, or WEBP.
  Images shold be no bigger than 200px &times; 200px, to not take up too
  much space on the page. 
</p>

With this kind of information on the page, I think many user errors can be prevented.

Ah, and another issue - when I add or change the game box image on a project page and press the ✓ I get the error

APIError: 500 Internal Server Error: error returned from database: (code: 787) FOREIGN KEY constraint failed

but if I click ✓ again, then all is good and the image is set or changed. Whether this is due to the project_id or user_id foreign key, I cannot tell. This does not happen when editing other fields in the GameSection interface.

Yours,
Christian