From b4ce7bc1f12ee62a59e7879be61f0ff76af4a2b3 Mon Sep 17 00:00:00 2001 From: Tad Hardesty Date: Thu, 16 Aug 2018 01:09:21 -0700 Subject: [PATCH] Validate map templates uploaded by admins (#39674) * Move the preloader datum to its own file * Prettify some of the map loader docs * Use src rather than usr in map template verbs * Cache parsed templates between upload and first use * Validate map templates uploaded by admins before use * Add href token to validation report links --- .../admin/verbs/map_template_loadverb.dm | 49 ++++++---- code/modules/mapping/map_template.dm | 21 ++-- code/modules/mapping/preloader.dm | 31 ++++++ code/modules/mapping/reader.dm | 60 +++--------- code/modules/mapping/verify.dm | 98 +++++++++++++++++++ tgstation.dme | 2 + 6 files changed, 191 insertions(+), 70 deletions(-) create mode 100644 code/modules/mapping/preloader.dm create mode 100644 code/modules/mapping/verify.dm diff --git a/code/modules/admin/verbs/map_template_loadverb.dm b/code/modules/admin/verbs/map_template_loadverb.dm index 22402a8c0d1..c5c9a0daf4b 100644 --- a/code/modules/admin/verbs/map_template_loadverb.dm +++ b/code/modules/admin/verbs/map_template_loadverb.dm @@ -4,7 +4,7 @@ var/datum/map_template/template - var/map = input(usr, "Choose a Map Template to place at your CURRENT LOCATION","Place Map Template") as null|anything in SSmapping.map_templates + var/map = input(src, "Choose a Map Template to place at your CURRENT LOCATION","Place Map Template") as null|anything in SSmapping.map_templates if(!map) return template = SSmapping.map_templates[map] @@ -18,35 +18,50 @@ var/image/item = image('icons/turf/overlays.dmi',S,"greenOverlay") item.plane = ABOVE_LIGHTING_PLANE preview += item - usr.client.images += preview - if(alert(usr,"Confirm location.","Template Confirm","Yes","No") == "Yes") + images += preview + if(alert(src,"Confirm location.","Template Confirm","Yes","No") == "Yes") if(template.load(T, centered = TRUE)) - message_admins("[key_name_admin(usr)] has placed a map template ([template.name]) at [ADMIN_COORDJMP(T)]") + message_admins("[key_name_admin(src)] has placed a map template ([template.name]) at [ADMIN_COORDJMP(T)]") else - to_chat(usr, "Failed to place map") - usr.client.images -= preview + to_chat(src, "Failed to place map") + images -= preview /client/proc/map_template_upload() set category = "Debug" set name = "Map Template - Upload" - var/map = input(usr, "Choose a Map Template to upload to template storage","Upload Map Template") as null|file + var/map = input(src, "Choose a Map Template to upload to template storage","Upload Map Template") as null|file if(!map) return if(copytext("[map]",-4) != ".dmm") - to_chat(usr, "Bad map file: [map]") + to_chat(src, "Filename must end in '.dmm': [map]") return var/datum/map_template/M - switch(alert(usr, "What kind of map is this?", "Map type", "Normal", "Shuttle", "Cancel")) + switch(alert(src, "What kind of map is this?", "Map type", "Normal", "Shuttle", "Cancel")) if("Normal") - M = new /datum/map_template(map, "[map]") + M = new /datum/map_template(map, "[map]", TRUE) if("Shuttle") - M = new /datum/map_template/shuttle(map, "[map]") + M = new /datum/map_template/shuttle(map, "[map]", TRUE) else return - if(M.preload_size(map)) - to_chat(usr, "Map template '[map]' ready to place ([M.width]x[M.height])") - SSmapping.map_templates[M.name] = M - message_admins("[key_name_admin(usr)] has uploaded a map template ([map])") - else - to_chat(usr, "Map template '[map]' failed to load properly") + if(!M.cached_map) + to_chat(src, "Map template '[map]' failed to parse properly.") + return + + var/datum/map_report/report = M.cached_map.check_for_errors() + var/report_link + if(report) + report.show_to(src) + report_link = " - validation report" + to_chat(src, "Map template '[map]' failed validation.") + if(report.loadable) + var/response = alert(src, "The map failed validation, would you like to load it anyways?", "Map Errors", "Cancel", "Upload Anyways") + if(response != "Upload Anyways") + return + else + alert(src, "The map failed validation and cannot be loaded.", "Map Errors", "Oh Darn") + return + + SSmapping.map_templates[M.name] = M + message_admins("[key_name_admin(src)] has uploaded a map template '[map]' ([M.width]x[M.height])[report_link].") + to_chat(src, "Map template '[map]' ready to place ([M.width]x[M.height])") diff --git a/code/modules/mapping/map_template.dm b/code/modules/mapping/map_template.dm index df8423ad66d..ceba29baecd 100644 --- a/code/modules/mapping/map_template.dm +++ b/code/modules/mapping/map_template.dm @@ -4,21 +4,25 @@ var/height = 0 var/mappath = null var/loaded = 0 // Times loaded this round + var/datum/parsed_map/cached_map + var/keep_cached_map = FALSE -/datum/map_template/New(path = null, rename = null) +/datum/map_template/New(path = null, rename = null, cache = FALSE) if(path) mappath = path if(mappath) - preload_size(mappath) + preload_size(mappath, cache) if(rename) name = rename -/datum/map_template/proc/preload_size(path) - var/datum/parsed_map/parsed = load_map(file(path), 1, 1, 1, cropMap=FALSE, measureOnly=TRUE) +/datum/map_template/proc/preload_size(path, cache = FALSE) + var/datum/parsed_map/parsed = new(file(path)) var/bounds = parsed?.bounds if(bounds) width = bounds[MAP_MAXX] // Assumes all templates are rectangular, have a single Z level, and begin at 1,1,1 height = bounds[MAP_MAXY] + if(cache) + cached_map = parsed return bounds /datum/parsed_map/proc/initTemplateBounds() @@ -77,8 +81,13 @@ if(T.y+height > world.maxy) return - var/datum/parsed_map/parsed = load_map(file(mappath), T.x, T.y, T.z, cropMap=TRUE, no_changeturf=(SSatoms.initialized == INITIALIZATION_INSSATOMS), placeOnTop=TRUE) - var/list/bounds = parsed?.bounds + // Accept cached maps, but don't save them automatically - we don't want + // ruins clogging up memory for the whole round. + var/datum/parsed_map/parsed = cached_map || new(file(mappath)) + cached_map = keep_cached_map ? parsed : null + if(!parsed.load(T.x, T.y, T.z, cropMap=TRUE, no_changeturf=(SSatoms.initialized == INITIALIZATION_INSSATOMS), placeOnTop=TRUE)) + return + var/list/bounds = parsed.bounds if(!bounds) return diff --git a/code/modules/mapping/preloader.dm b/code/modules/mapping/preloader.dm new file mode 100644 index 00000000000..e6fa2421a02 --- /dev/null +++ b/code/modules/mapping/preloader.dm @@ -0,0 +1,31 @@ +// global datum that will preload variables on atoms instanciation +GLOBAL_VAR_INIT(use_preloader, FALSE) +GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) + +/// Preloader datum +/datum/map_preloader + parent_type = /datum + var/list/attributes + var/target_path + +/datum/map_preloader/proc/setup(list/the_attributes, path) + if(the_attributes.len) + GLOB.use_preloader = TRUE + attributes = the_attributes + target_path = path + +/datum/map_preloader/proc/load(atom/what) + GLOB.use_preloader = FALSE + for(var/attribute in attributes) + var/value = attributes[attribute] + if(islist(value)) + value = deepCopyList(value) + what.vars[attribute] = value + +/area/template_noop + name = "Area Passthrough" + +/turf/template_noop + name = "Turf Passthrough" + icon_state = "noop" + bullet_bounce_sound = null diff --git a/code/modules/mapping/reader.dm b/code/modules/mapping/reader.dm index 5b7ae335568..2889a6c932a 100644 --- a/code/modules/mapping/reader.dm +++ b/code/modules/mapping/reader.dm @@ -2,9 +2,6 @@ //SS13 Optimized Map loader ////////////////////////////////////////////////////////////// #define SPACE_KEY "space" -//global datum that will preload variables on atoms instanciation -GLOBAL_VAR_INIT(use_preloader, FALSE) -GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) /datum/grid_set var/xcrd @@ -19,7 +16,6 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) var/list/gridSets = list() var/list/modelCache - var/list/bad_paths /// Unoffset bounds. Null on parse failure. var/list/parsed_bounds @@ -38,13 +34,13 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) /// Shortcut function to parse a map and apply it to the world. /// -/// - dmm_file: A .dmm file to load (Required). -/// - x_offset, y_offset, z_offset: Positions representign where to load the map (Optional). -/// - cropMap: When true, the map will be cropped to fit the existing world dimensions (Optional). -/// - measureOnly: When true, no changes will be made to the world (Optional). -/// - no_changeturf: When true, turf/AfterChange won't be called on loaded turfs -/// - x_lower, x_upper, y_lower, y_upper: Coordinates (relative to the map) to crop to (Optional). -/// - placeOnTop: Whether to use turf/PlaceOnTop rather than turf/ChangeTurf (Optional). +/// - `dmm_file`: A .dmm file to load (Required). +/// - `x_offset`, `y_offset`, `z_offset`: Positions representign where to load the map (Optional). +/// - `cropMap`: When true, the map will be cropped to fit the existing world dimensions (Optional). +/// - `measureOnly`: When true, no changes will be made to the world (Optional). +/// - `no_changeturf`: When true, [turf/AfterChange] won't be called on loaded turfs +/// - `x_lower`, `x_upper`, `y_lower`, `y_upper`: Coordinates (relative to the map) to crop to (Optional). +/// - `placeOnTop`: Whether to use [turf/PlaceOnTop] rather than [turf/ChangeTurf] (Optional). /proc/load_map(dmm_file as file, x_offset as num, y_offset as num, z_offset as num, cropMap as num, measureOnly as num, no_changeturf as num, x_lower = -INFINITY as num, x_upper = INFINITY as num, y_lower = -INFINITY as num, y_upper = INFINITY as num, placeOnTop = FALSE as num) var/datum/parsed_map/parsed = new(dmm_file, x_lower, x_upper, y_lower, y_upper, measureOnly) if(parsed.bounds && !measureOnly) @@ -133,7 +129,7 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) bounds = null parsed_bounds = bounds -/// Load the parsed map into the world. See /proc/load_map for arguments. +/// Load the parsed map into the world. See [/proc/load_map] for arguments. /datum/parsed_map/proc/load(x_offset, y_offset, z_offset, cropMap, no_changeturf, x_lower, x_upper, y_lower, y_upper, placeOnTop) //How I wish for RAII Master.StartLoadingMap() @@ -218,8 +214,8 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) return TRUE -/datum/parsed_map/proc/build_cache(no_changeturf) - if(modelCache) +/datum/parsed_map/proc/build_cache(no_changeturf, bad_paths=null) + if(modelCache && !bad_paths) return modelCache . = modelCache = list() var/list/grid_models = src.grid_models @@ -246,8 +242,9 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) var/atom_def = text2path(path_text) //path definition, e.g /obj/foo/bar old_position = dpos + 1 - if(!atom_def) // Skip the item if the path does not exist. Fix your crap, mappers! - LAZYADD(bad_paths, path_text) + if(!ispath(atom_def, /atom)) // Skip the item if the path does not exist. Fix your crap, mappers! + if(bad_paths) + LAZYOR(bad_paths[path_text], model_key) continue members.Add(atom_def) @@ -467,34 +464,3 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new) /datum/parsed_map/Destroy() ..() return QDEL_HINT_HARDDEL_NOW - -////////////////// -//Preloader datum -////////////////// - -/datum/map_preloader - parent_type = /datum - var/list/attributes - var/target_path - -/datum/map_preloader/proc/setup(list/the_attributes, path) - if(the_attributes.len) - GLOB.use_preloader = TRUE - attributes = the_attributes - target_path = path - -/datum/map_preloader/proc/load(atom/what) - GLOB.use_preloader = FALSE - for(var/attribute in attributes) - var/value = attributes[attribute] - if(islist(value)) - value = deepCopyList(value) - what.vars[attribute] = value - -/area/template_noop - name = "Area Passthrough" - -/turf/template_noop - name = "Turf Passthrough" - icon_state = "noop" - bullet_bounce_sound = null diff --git a/code/modules/mapping/verify.dm b/code/modules/mapping/verify.dm new file mode 100644 index 00000000000..a9834e37098 --- /dev/null +++ b/code/modules/mapping/verify.dm @@ -0,0 +1,98 @@ +/// An error report generated by [parsed_map/check_for_errors]. +/datum/map_report + var/original_path + var/list/bad_paths = list() + var/list/bad_keys = list() + /// Whether this map can be loaded safely despite the errors. + var/loadable = TRUE + var/crashed = TRUE + + var/static/tag_number = 0 + +/datum/map_report/New(datum/parsed_map/map) + original_path = map.original_path || "Untitled" + +/// Show a rendered version of this report to a client. +/datum/map_report/proc/show_to(client/C) + var/list/html = list() + html += "

Report for map file [original_path]

" + if(crashed) + html += "

Validation crashed: check the runtime logs.

" + if(!loadable) + html += "

Not loadable: some tiles are missing their turfs or areas.

" + + if(bad_paths.len) + html += "

Bad paths:

    " + for(var/path in bad_paths) + var/list/keys = bad_paths[path] + html += "
  1. [path]: used in ([keys.len]): [keys.Join(", ")]" + html += "

" + + if(bad_keys.len) + html += "

Bad keys:

" + C << browse(html.Join(), "window=[tag];size=600x400") + +/datum/map_report/Topic(href, href_list) + . = ..() + if(. || !check_rights(R_ADMIN, FALSE) || !usr.client.holder.CheckAdminHref(href, href_list)) + return + + if (href_list["show"]) + show_to(usr) + + +/// Check a parsed but not yet loaded map for errors. +/// +/// Returns a [/datum/map_report] if there are errors or `FALSE` otherwise. +/datum/parsed_map/proc/check_for_errors() + var/datum/map_report/report = new(src) + . = report + + // build_cache will check bad paths for us + var/list/modelCache = build_cache(TRUE, report.bad_paths) + + for(var/path in report.bad_paths) + if(copytext(path, 1, 7) == "/turf/" || copytext(path, 1, 7) == "/area/") + report.loadable = FALSE + + // check for tiles with the wrong number of turfs or areas + for(var/key in modelCache) + if(key == SPACE_KEY) + continue + var/model = modelCache[key] + var/list/members = model[1] + + var/turfs = 0 + var/areas = 0 + for(var/i in 1 to members.len) + var/atom/path = members[i] + + turfs += ispath(path, /turf) + areas += ispath(path, /area) + + if(turfs == 0) + report.loadable = FALSE + LAZYADD(report.bad_keys[key], "no turf") + else if(turfs > 1) + LAZYADD(report.bad_keys[key], "[turfs] stacked turfs") + + if(areas != 1) + report.loadable = FALSE + LAZYADD(report.bad_keys[key], "[areas] areas instead of 1") + + // return the report + if(report.bad_paths.len || report.bad_keys.len || !report.loadable) + // keep the report around so it can be referenced later + report.tag = "mapreport_[++report.tag_number]" + report.crashed = FALSE + else + return FALSE diff --git a/tgstation.dme b/tgstation.dme index 1eb1be47e23..bc342fd6a2a 100755 --- a/tgstation.dme +++ b/tgstation.dme @@ -1745,8 +1745,10 @@ #include "code\modules\lighting\lighting_turf.dm" #include "code\modules\mapping\map_template.dm" #include "code\modules\mapping\mapping_helpers.dm" +#include "code\modules\mapping\preloader.dm" #include "code\modules\mapping\reader.dm" #include "code\modules\mapping\ruins.dm" +#include "code\modules\mapping\verify.dm" #include "code\modules\mapping\space_management\space_level.dm" #include "code\modules\mapping\space_management\space_reservation.dm" #include "code\modules\mapping\space_management\space_transition.dm"