Fix 404 race when uploading attachments
This commit is contained in:
		
							parent
							
								
									5724bdf436
								
							
						
					
					
						commit
						cfcc3793c5
					
				
					 2 changed files with 29 additions and 2 deletions
				
			
		|  | @ -41,7 +41,6 @@ import ( | ||||||
|   - tokens |   - tokens | ||||||
| - HIGH Self-review | - HIGH Self-review | ||||||
| - MEDIUM: Test for expiring messages after reservation removal | - MEDIUM: Test for expiring messages after reservation removal | ||||||
| - MEDIUM: uploading attachments leads to 404 -- race |  | ||||||
| - MEDIUM: Test new token endpoints & never-expiring token | - MEDIUM: Test new token endpoints & never-expiring token | ||||||
| - LOW: UI: Flickering upgrade banner when logging in | - LOW: UI: Flickering upgrade banner when logging in | ||||||
| - LOW: Menu item -> popup click should not open page | - LOW: Menu item -> popup click should not open page | ||||||
|  | @ -563,7 +562,16 @@ func (s *Server) handleFile(w http.ResponseWriter, r *http.Request, v *visitor) | ||||||
| 	//   - and also uses the higher bandwidth limits of a paying user | 	//   - and also uses the higher bandwidth limits of a paying user | ||||||
| 	m, err := s.messageCache.Message(messageID) | 	m, err := s.messageCache.Message(messageID) | ||||||
| 	if err == errMessageNotFound { | 	if err == errMessageNotFound { | ||||||
|  | 		if s.config.CacheBatchTimeout > 0 { | ||||||
|  | 			// Strange edge case: If we immediately after upload request the file (the web app does this for images), | ||||||
|  | 			// and messages are persisted asynchronously, retry fetching from the database | ||||||
|  | 			m, err = util.Retry(func() (*message, error) { | ||||||
|  | 				return s.messageCache.Message(messageID) | ||||||
|  | 			}, s.config.CacheBatchTimeout, 100*time.Millisecond, 300*time.Millisecond, 600*time.Millisecond) | ||||||
|  | 		} | ||||||
|  | 		if err != nil { | ||||||
| 			return errHTTPNotFound | 			return errHTTPNotFound | ||||||
|  | 		} | ||||||
| 	} else if err != nil { | 	} else if err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -1714,6 +1714,25 @@ func TestServer_PublishAttachmentBandwidthLimitUploadOnly(t *testing.T) { | ||||||
| 	require.Equal(t, 41301, err.Code) | 	require.Equal(t, 41301, err.Code) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | func TestServer_PublishAttachmentAndImmediatelyGetItWithCacheTimeout(t *testing.T) { | ||||||
|  | 	// This tests the awkward util.Retry in handleFile: Due to the async persisting of messages, | ||||||
|  | 	// the message is not immediately available when attempting to download it. | ||||||
|  | 
 | ||||||
|  | 	c := newTestConfig(t) | ||||||
|  | 	c.CacheBatchTimeout = 500 * time.Millisecond | ||||||
|  | 	c.CacheBatchSize = 10 | ||||||
|  | 	s := newTestServer(t, c) | ||||||
|  | 	content := "this is an ATTACHMENT" | ||||||
|  | 	rr := request(t, s, "PUT", "/mytopic?f=myfile.txt", content, nil) | ||||||
|  | 	m := toMessage(t, rr.Body.String()) | ||||||
|  | 	require.Equal(t, "myfile.txt", m.Attachment.Name) | ||||||
|  | 
 | ||||||
|  | 	path := strings.TrimPrefix(m.Attachment.URL, "http://127.0.0.1:12345") | ||||||
|  | 	rr = request(t, s, "GET", path, "", nil) | ||||||
|  | 	require.Equal(t, 200, rr.Code) // Not 404! | ||||||
|  | 	require.Equal(t, content, rr.Body.String()) | ||||||
|  | } | ||||||
|  | 
 | ||||||
| func TestServer_PublishAttachmentAccountStats(t *testing.T) { | func TestServer_PublishAttachmentAccountStats(t *testing.T) { | ||||||
| 	content := util.RandomString(4999) // > 4096 | 	content := util.RandomString(4999) // > 4096 | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue