weblegacy / struts1

Struts1-Upgrade to current technology
https://weblegacy.github.io/struts1/
Apache License 2.0
31 stars 5 forks source link

html:cancel taglib not working in case of MultipartRequestWrapper #32

Open nrmnrm opened 4 months ago

nrmnrm commented 4 months ago

If I cancel an action using taglib html:cancel on a jsp page it is not propagated to the action if coming from a multipart request, for standard requests its fine (see the two sceenshots from debug mode attached)

working

not_working

nrmnrm commented 3 months ago

HI Stefan,

can I help you in any ways to tackle that?

ste-gr commented 3 months ago

Hi @nrmnrm,

sorry for the long break. I don't have much time at the moment. Maybe you can find a solution and I can have a look at it and release it.

Greetings Stefan

nrmnrm commented 2 months ago

Took me some hours, but I have found it :) Please check AbstractPopulateActionForm.java and change order in execute_ method, cancel needs to be executed last, as it requires population first to work:

protected boolean execute_(ActionContext actionCtx) throws Exception {

    ActionConfig actionConfig = actionCtx.getActionConfig();
    ActionForm actionForm = actionCtx.getActionForm();

    // Is there a form bean for this request?
    if (actionForm == null) {
        return (false);
    }

    // Reset the form bean only if configured so
    if (isReset(actionCtx, actionConfig)) {
        log.debug("Reseting form bean '{}'",  actionConfig.getName());
        reset(actionCtx, actionConfig, actionForm);
    }

    // Populate the form bean only if configured so
    if (isPopulate(actionCtx, actionConfig)) {
        log.debug("Populating form bean '{}'", actionConfig.getName());
        populate(actionCtx, actionConfig, actionForm);
    }

// Always handle cancel last, as it needs populate first
    handleCancel(actionCtx, actionConfig, actionForm);

    return CONTINUE_PROCESSING;
}

Not sure when and why changing of the order was introduced but it definitely breaks html:cancel on mulitpart requests: I have rebuild with that change and it works fine for me now.

Best regards!

nrmnrm commented 2 months ago

I think this commit broke it "STR-286 and STR-1116: ActionForm populate and reset strategy"