Skip to content
Permalink
Browse files

Fix a bug where user could be redirected to sign in while uploading a…

… large file. Fixes #5202
  • Loading branch information...
kfeinauer committed Aug 9, 2019
1 parent 0429753 commit fbb5aea45e359210beb099d04691d2a66ce99d44
@@ -396,6 +396,17 @@ void Response::addCookie(const Cookie& cookie)
return headers;
}

void Response::clearCookies()
{
for (auto iter = headers_.begin(); iter != headers_.end();)
{
if (iter->name == "Set-Cookie")
iter = headers_.erase(iter);
else
++iter;
}
}

Error Response::setBody(const std::string& content)
{
std::istringstream is(content);
@@ -153,7 +153,8 @@ class Response : public Message

void setBrowserCompatible(const Request& request);

void addCookie(const Cookie& cookie) ;
void addCookie(const Cookie& cookie);
void clearCookies();
Headers getCookies() const;

Error setBody(const std::string& content);
@@ -309,6 +309,9 @@ void refreshAuthCookies(const std::string& userIdentifier,
if (server::options().authTimeoutMinutes() > 0 &&
!s_handler.refreshAuthCookies.empty())
{
// clear any existing auth cookies first - this method can be invoked multiple
// times depending on the handler type (for example, an upload handler)
pResponse->clearCookies();
bool persist = request.cookieValue(kPersistAuthCookie) == "1" ? true : false;
s_handler.refreshAuthCookies(request, userIdentifier, persist, pResponse);
}
@@ -175,22 +175,26 @@ class AsyncUriHandler

bool getUser(boost::shared_ptr<http::AsyncConnection> pConnection)
{
if (!userIdentifier_.empty() && !username_.empty())
return true;

userIdentifier_ = handler::getUserIdentifier(pConnection->request(),
&pConnection->response());
if (userIdentifier_.empty())
// cache the username so we don't have to determine it every time this is invoked
// looking up the username and performing the secure cookie decode is expensive
// this optimization is important for large file uploads to prevent a lot of unnecessary work
if (userIdentifier_.empty() || username_.empty())
{
unauthorizedResponseFunction_(pConnection);
return false;
userIdentifier_ = handler::getUserIdentifier(pConnection->request(),
&pConnection->response());
if (userIdentifier_.empty())
{
unauthorizedResponseFunction_(pConnection);
return false;
}

// convert to local username
username_ = handler::userIdentifierToLocalUsername(userIdentifier_);
}

if (refreshAuthCookies_)
handler::refreshAuthCookies(userIdentifier_, pConnection->request(), &pConnection->response());

// convert to local username
username_ = handler::userIdentifierToLocalUsername(userIdentifier_);
return true;
}

@@ -35,7 +35,9 @@ public HtmlFormModalDialog(String title,
DialogRole role,
final String progressMessage,
String actionURL,
final OperationWithInput<T> operation)
final Operation beginOperation,
final OperationWithInput<T> completedOperation,
final Operation failedOperation)
{
super(new FormPanel(), role);
setText(title);
@@ -47,12 +49,42 @@ public HtmlFormModalDialog(String title,
setFormPanelEncodingAndMethod(formPanel);

final ProgressIndicator progressIndicator = addProgressIndicator();
final ProgressIndicator indicatorWrapper = new ProgressIndicator()
{
public void onProgress(String message)
{
progressIndicator.onProgress(message);
}

public void onProgress(String message, Operation onCancel)
{
progressIndicator.onProgress(message, onCancel);
}

public void onCompleted()
{
progressIndicator.onCompleted();
}

public void onError(String message)
{
progressIndicator.onError(message);
failedOperation.execute();
}

@Override
public void clearProgress()
{
progressIndicator.clearProgress();
}
};

ThemedButton okButton = new ThemedButton("OK", new ClickHandler() {
public void onClick(ClickEvent event) {
try
{
formPanel.submit();
beginOperation.execute();
}
catch (final JavaScriptException e)
{
@@ -63,13 +95,13 @@ public void execute()
if ("Access is denied.".equals(
StringUtil.notNull(e.getDescription()).trim()))
{
progressIndicator.onError(
indicatorWrapper.onError(
"Please use a complete file path.");
}
else
{
Debug.log(e.toString());
progressIndicator.onError(e.getDescription());
indicatorWrapper.onError(e.getDescription());
}
}
});
@@ -81,7 +113,7 @@ public void execute()
public void execute()
{
Debug.log(e.toString());
progressIndicator.onError(e.getMessage());
indicatorWrapper.onError(e.getMessage());
}
});
}
@@ -105,7 +137,7 @@ public void onClick(ClickEvent event)
public void onSubmit(SubmitEvent event) {
if (validate())
{
progressIndicator.onProgress(progressMessage);
indicatorWrapper.onProgress(progressMessage);
}
else
{
@@ -123,18 +155,17 @@ public void onSubmitComplete(SubmitCompleteEvent event) {
try
{
T results = parseResults(resultsText);
progressIndicator.onCompleted();
operation.execute(results);
indicatorWrapper.onCompleted();
completedOperation.execute(results);
}
catch(Exception e)
{
progressIndicator.onError(e.getMessage());
indicatorWrapper.onError(e.getMessage());
}
}
else
{
progressIndicator.onError(
"Unexpected empty response from server");
indicatorWrapper.onError("Unexpected empty response from server");
}
}
});
@@ -157,6 +157,7 @@ public Application(ApplicationView view,
events.addHandler(InvalidSessionEvent.TYPE, this);
events.addHandler(SwitchToRVersionEvent.TYPE, this);
events.addHandler(SessionInitEvent.TYPE, this);
events.addHandler(FileUploadEvent.TYPE, this);

// register for uncaught exceptions
uncaughtExHandler.register();
@@ -326,8 +327,19 @@ void onShowSessionServerOptionsDialog()

public void onUnauthorized(UnauthorizedEvent event)
{
navigateToSignIn();
}
// if the user is currently uploading a file (which potentially takes a long time)
// and we were to navigate them, they would be unable to complete the upload
if (!fileUploadInProgress_)
{
server_.disconnect();
navigateToSignIn();
}
}

public void onFileUpload(FileUploadEvent event)
{
fileUploadInProgress_ = event.inProgress();
}

public void onServerOffline(ServerOfflineEvent event)
{
@@ -1240,6 +1252,8 @@ private void resumeClientStateUpdater()
private final Provider<ApplicationInterrupt> pApplicationInterrupt_;
private final Provider<ProductEditionInfo> pEdition_;
private final Provider<ApplicationThemes> pAppThemes_;

private boolean fileUploadInProgress_ = false;

private final String CSRF_TOKEN_FIELD = "csrf-token";

@@ -32,6 +32,7 @@
ServerOfflineHandler,
InvalidSessionEvent.Handler,
SessionInitHandler,
RestartStatusEvent.Handler
RestartStatusEvent.Handler,
FileUploadHandler
{
}
@@ -2961,10 +2961,6 @@ RpcRequest getEvents(

void handleUnauthorizedError()
{
// disconnect
disconnect();

// fire event
UnauthorizedEvent event = new UnauthorizedEvent();
eventBus_.fireEvent(event);
}
@@ -116,7 +116,9 @@ void showFileUpload(
String targetURL,
FileSystemItem targetDirectory,
RemoteFileSystemContext fileSystemContext,
OperationWithInput<PendingFileUpload> completedOperation);
Operation beginOperation,
OperationWithInput<PendingFileUpload> completedOperation,
Operation failedOperation);


void showHtmlFileChoice(FileSystemItem file,
@@ -31,6 +31,7 @@
import org.rstudio.core.client.command.AppCommand;
import org.rstudio.core.client.files.FileSystemItem;
import org.rstudio.core.client.resources.ImageResource2x;
import org.rstudio.core.client.widget.Operation;
import org.rstudio.core.client.widget.OperationWithInput;
import org.rstudio.core.client.widget.ProgressOperationWithInput;
import org.rstudio.core.client.widget.Toolbar;
@@ -206,13 +207,17 @@ public void showFileUpload(
String targetURL,
FileSystemItem targetDirectory,
RemoteFileSystemContext fileSystemContext,
OperationWithInput<PendingFileUpload> completedOperation)
Operation beginOperation,
OperationWithInput<PendingFileUpload> completedOperation,
Operation failedOperation)
{
FileUploadDialog dlg = new FileUploadDialog(targetURL,
targetDirectory,
fileDialogs_,
fileSystemContext,
completedOperation);
beginOperation,
completedOperation,
failedOperation);
dlg.showModal();
}

@@ -21,6 +21,8 @@
import org.rstudio.core.client.widget.MessageDialog;
import org.rstudio.core.client.widget.Operation;
import org.rstudio.core.client.widget.OperationWithInput;
import org.rstudio.studio.client.application.events.EventBus;
import org.rstudio.studio.client.application.events.FileUploadEvent;
import org.rstudio.studio.client.common.GlobalDisplay;
import org.rstudio.studio.client.server.ServerError;
import org.rstudio.studio.client.server.ServerRequestCallback;
@@ -35,11 +37,13 @@
@Inject
public FilesUpload(Files.Display display,
GlobalDisplay globalDisplay,
FilesServerOperations server)
FilesServerOperations server,
EventBus eventBus)
{
display_ = display;
globalDisplay_ = globalDisplay;
server_ = server;
eventBus_ = eventBus;
}

void execute(FileSystemItem targetDirectory,
@@ -53,6 +57,13 @@ void execute(FileSystemItem targetDirectory,
server_.getFileUploadUrl(),
targetDirectory,
fileSystemContext,
new Operation() {
public void execute()
{
FileUploadEvent event = new FileUploadEvent(true);
eventBus_.fireEvent(event);
}
},
new OperationWithInput<PendingFileUpload>() {
public void execute(PendingFileUpload pendingUpload)
{
@@ -74,7 +85,14 @@ public void execute(PendingFileUpload pendingUpload)
completeFileUploadOperation(token, true).execute();
}
}
});
},
new Operation() {
public void execute()
{
FileUploadEvent event = new FileUploadEvent(false);
eventBus_.fireEvent(event);
}
} );
}


@@ -118,6 +136,9 @@ public void execute()
public void onResponseReceived(Void response)
{
dismissProgress.execute();

FileUploadEvent event = new FileUploadEvent(false);
eventBus_.fireEvent(event);
}

@Override
@@ -126,6 +147,9 @@ public void onError(ServerError error)
dismissProgress.execute();
globalDisplay_.showErrorMessage("File Upload Error",
error.getUserMessage());

FileUploadEvent event = new FileUploadEvent(false);
eventBus_.fireEvent(event);
}
});
}
@@ -136,4 +160,5 @@ public void onError(ServerError error)
private final Files.Display display_;
private final GlobalDisplay globalDisplay_ ;
private final FilesServerOperations server_ ;
private final EventBus eventBus_;
}
@@ -37,6 +37,7 @@
import org.rstudio.core.client.widget.DecorativeImage;
import org.rstudio.core.client.widget.FormLabel;
import org.rstudio.core.client.widget.HtmlFormModalDialog;
import org.rstudio.core.client.widget.Operation;
import org.rstudio.core.client.widget.OperationWithInput;
import org.rstudio.core.client.widget.ProgressIndicator;
import org.rstudio.core.client.widget.ProgressOperationWithInput;
@@ -52,13 +53,17 @@ public FileUploadDialog(
FileSystemItem targetDirectory,
FileDialogs fileDialogs,
RemoteFileSystemContext fileSystemContext,
OperationWithInput<PendingFileUpload> completedOperation)
Operation beginOperation,
OperationWithInput<PendingFileUpload> completedOperation,
Operation failedOperation)
{
super("Upload Files",
Roles.getDialogRole(),
"Uploading file...",
actionURL,
completedOperation);
beginOperation,
completedOperation,
failedOperation);
fileDialogs_ = fileDialogs;
fileSystemContext_ = fileSystemContext;
targetDirectory_ = targetDirectory;

0 comments on commit fbb5aea

Please sign in to comment.
You can’t perform that action at this time.